Skip to content

Commit f484fba

Browse files
committed
femtovg: Fix rendering of children outside a layer's intrinsic bounding rect
The layer clips (by being a texture), so we must make sure to size it beyond its intrinsic size and include the bounding rect of the children, in size **and** position. cc #9808
1 parent 5ebdc50 commit f484fba

File tree

1 file changed

+84
-77
lines changed

1 file changed

+84
-77
lines changed

internal/renderers/femtovg/itemrenderer.rs

Lines changed: 84 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ pub type CanvasRc<R> = Rc<RefCell<Canvas<R>>>;
3838

3939
pub enum ItemGraphicsCacheEntry<R: femtovg::Renderer + TextureImporter> {
4040
Texture(Rc<Texture<R>>),
41+
TextureWithOrigin {
42+
texture: Rc<Texture<R>>,
43+
origin: PhysicalPoint,
44+
},
4145
ColorizedImage {
4246
// This original image Rc is kept here to keep the image in the shared image cache, so that
4347
// changes to the colorization brush will not require re-uploading the image.
@@ -50,6 +54,9 @@ impl<R: femtovg::Renderer + TextureImporter> Clone for ItemGraphicsCacheEntry<R>
5054
fn clone(&self) -> Self {
5155
match self {
5256
Self::Texture(arg0) => Self::Texture(arg0.clone()),
57+
Self::TextureWithOrigin { texture, origin } => {
58+
Self::TextureWithOrigin { texture: texture.clone(), origin: origin.clone() }
59+
}
5360
Self::ColorizedImage { _original_image, colorized_image } => Self::ColorizedImage {
5461
_original_image: _original_image.clone(),
5562
colorized_image: colorized_image.clone(),
@@ -62,6 +69,7 @@ impl<R: femtovg::Renderer + TextureImporter> ItemGraphicsCacheEntry<R> {
6269
fn as_texture(&self) -> &Rc<Texture<R>> {
6370
match self {
6471
ItemGraphicsCacheEntry::Texture(image) => image,
72+
ItemGraphicsCacheEntry::TextureWithOrigin { texture, .. } => texture,
6573
ItemGraphicsCacheEntry::ColorizedImage { colorized_image, .. } => colorized_image,
6674
}
6775
}
@@ -670,7 +678,9 @@ impl<'a, R: femtovg::Renderer + TextureImporter> ItemRenderer for GLItemRenderer
670678
let border_width = clip_item.border_width();
671679

672680
if !radius.is_zero() {
673-
if let Some(layer_image) = self.render_layer(item_rc, &|| item_rc.geometry()) {
681+
if let Some((layer_origin, layer_image)) =
682+
self.render_layer(item_rc, &|| item_rc.geometry())
683+
{
674684
let layer_image_paint = layer_image.as_paint();
675685

676686
let layer_path = clip_path_for_rect_alike_item(
@@ -769,6 +779,7 @@ impl<'a, R: femtovg::Renderer + TextureImporter> ItemRenderer for GLItemRenderer
769779
});
770780
let image_id = match cache_entry {
771781
Some(ItemGraphicsCacheEntry::Texture(image)) => image.id,
782+
Some(ItemGraphicsCacheEntry::TextureWithOrigin { texture, .. }) => texture.id,
772783
Some(ItemGraphicsCacheEntry::ColorizedImage { .. }) => unreachable!(),
773784
None => return,
774785
};
@@ -1039,114 +1050,110 @@ impl<'a, R: femtovg::Renderer + TextureImporter> GLItemRenderer<'a, R> {
10391050
&mut self,
10401051
item_rc: &ItemRc,
10411052
layer_bounding_rect_fn: &dyn Fn() -> LogicalRect,
1042-
) -> Option<Rc<Texture<R>>> {
1053+
) -> Option<(PhysicalPoint, Rc<Texture<R>>)> {
10431054
let existing_layer_texture =
10441055
self.graphics_cache.with_entry(item_rc, |cache_entry| match cache_entry {
1045-
Some(ItemGraphicsCacheEntry::Texture(texture)) => Some(texture.clone()),
1056+
Some(ItemGraphicsCacheEntry::TextureWithOrigin { texture, .. }) => {
1057+
Some(texture.clone())
1058+
}
10461059
_ => None,
10471060
});
10481061

10491062
let cache_entry = self.graphics_cache.get_or_update_cache_entry(item_rc, || {
1050-
ItemGraphicsCacheEntry::Texture({
1051-
let bounding_rect = layer_bounding_rect_fn();
1052-
let size = (bounding_rect.size * self.scale_factor).ceil().try_cast()?;
1053-
1054-
let layer_image = existing_layer_texture
1055-
.and_then(|layer_texture| {
1056-
// If we have an existing layer texture, there must be only one reference from within
1057-
// the existing cache entry and one through the `existing_layer_texture` variable.
1058-
// Then it is safe to render new content into it in this callback and when we return
1059-
// into `get_or_update_cache_entry` the first ref is dropped.
1060-
debug_assert_eq!(Rc::strong_count(&layer_texture), 2);
1061-
if layer_texture.size() == Some(size.to_untyped()) {
1062-
Some(layer_texture)
1063-
} else {
1064-
None
1065-
}
1066-
})
1067-
.or_else(|| {
1068-
*self.metrics.layers_created.as_mut().unwrap() += 1;
1069-
Texture::new_empty_on_gpu(&self.canvas, size.width, size.height)
1070-
})?;
1063+
let bounding_rect = layer_bounding_rect_fn();
1064+
let origin = bounding_rect.origin * self.scale_factor;
1065+
let size = (bounding_rect.size * self.scale_factor).ceil().try_cast()?;
1066+
1067+
let layer_image = existing_layer_texture
1068+
.and_then(|layer_texture| {
1069+
// If we have an existing layer texture, there must be only one reference from within
1070+
// the existing cache entry and one through the `existing_layer_texture` variable.
1071+
// Then it is safe to render new content into it in this callback and when we return
1072+
// into `get_or_update_cache_entry` the first ref is dropped.
1073+
debug_assert_eq!(Rc::strong_count(&layer_texture), 2);
1074+
if layer_texture.size() == Some(size.to_untyped()) {
1075+
Some(layer_texture)
1076+
} else {
1077+
None
1078+
}
1079+
})
1080+
.or_else(|| {
1081+
*self.metrics.layers_created.as_mut().unwrap() += 1;
1082+
Texture::new_empty_on_gpu(&self.canvas, size.width, size.height)
1083+
})?;
10711084

1072-
let previous_render_target = self.current_render_target();
1085+
let previous_render_target = self.current_render_target();
10731086

1074-
{
1075-
let mut canvas = self.canvas.borrow_mut();
1076-
canvas.save();
1087+
{
1088+
let mut canvas = self.canvas.borrow_mut();
1089+
canvas.save();
10771090

1078-
canvas.set_render_target(layer_image.as_render_target());
1091+
canvas.set_render_target(layer_image.as_render_target());
10791092

1080-
canvas.reset();
1093+
canvas.reset();
10811094

1082-
canvas.clear_rect(
1083-
0,
1084-
0,
1085-
size.width,
1086-
size.height,
1087-
femtovg::Color::rgba(0, 0, 0, 0),
1088-
);
1095+
canvas.clear_rect(0, 0, size.width, size.height, femtovg::Color::rgba(0, 0, 0, 0));
10891096

1090-
let origin = bounding_rect.origin * self.scale_factor;
1091-
canvas.translate(-origin.x, -origin.y);
1092-
}
1097+
canvas.translate(-origin.x, -origin.y);
1098+
}
10931099

1094-
*self.state.last_mut().unwrap() = State {
1095-
scissor: LogicalRect::new(LogicalPoint::default(), bounding_rect.size),
1096-
global_alpha: 1.,
1097-
current_render_target: layer_image.as_render_target(),
1098-
};
1100+
*self.state.last_mut().unwrap() = State {
1101+
scissor: bounding_rect,
1102+
global_alpha: 1.,
1103+
current_render_target: layer_image.as_render_target(),
1104+
};
10991105

1100-
let window_adapter = self.window().window_adapter();
1106+
let window_adapter = self.window().window_adapter();
11011107

1102-
i_slint_core::item_rendering::render_item_children(
1103-
self,
1104-
item_rc.item_tree(),
1105-
item_rc.index() as isize,
1106-
&window_adapter,
1107-
);
1108+
i_slint_core::item_rendering::render_item_children(
1109+
self,
1110+
item_rc.item_tree(),
1111+
item_rc.index() as isize,
1112+
&window_adapter,
1113+
);
11081114

1109-
{
1110-
let mut canvas = self.canvas.borrow_mut();
1111-
canvas.restore();
1115+
{
1116+
let mut canvas = self.canvas.borrow_mut();
1117+
canvas.restore();
11121118

1113-
canvas.set_render_target(previous_render_target);
1114-
}
1119+
canvas.set_render_target(previous_render_target);
1120+
}
11151121

1116-
layer_image
1117-
})
1118-
.into()
1122+
Some(ItemGraphicsCacheEntry::TextureWithOrigin { texture: layer_image, origin })
11191123
});
11201124

1121-
cache_entry.map(|item_cache_entry| item_cache_entry.as_texture().clone())
1125+
cache_entry.and_then(|item_cache_entry| match item_cache_entry {
1126+
ItemGraphicsCacheEntry::TextureWithOrigin { texture, origin } => {
1127+
Some((origin, texture.clone()))
1128+
}
1129+
_ => None,
1130+
})
11221131
}
11231132

11241133
fn render_and_blend_layer(&mut self, alpha_tint: f32, item_rc: &ItemRc) -> RenderingResult {
11251134
let current_clip = self.get_current_clip();
1126-
if let Some((layer_image, layer_size)) = self
1127-
.render_layer(item_rc, &|| {
1128-
// We don't need to include the size of the opacity item itself, since it has no content.
1129-
let children_rect = i_slint_core::properties::evaluate_no_tracking(|| {
1130-
item_rc.geometry().union(
1131-
&i_slint_core::item_rendering::item_children_bounding_rect(
1132-
item_rc.item_tree(),
1133-
item_rc.index() as isize,
1134-
&current_clip,
1135-
),
1136-
)
1137-
});
1138-
children_rect
1135+
if let Some((layer_origin, layer_image)) = self.render_layer(item_rc, &|| {
1136+
// We don't need to include the size of the opacity item itself, since it has no content.
1137+
i_slint_core::properties::evaluate_no_tracking(|| {
1138+
i_slint_core::item_rendering::item_children_bounding_rect(
1139+
item_rc.item_tree(),
1140+
item_rc.index() as isize,
1141+
&current_clip,
1142+
)
11391143
})
1140-
.and_then(|image| image.size().map(|size| (image, size)))
1144+
}) && let Some(layer_size) = layer_image.size()
11411145
{
11421146
let mut layer_path = femtovg::Path::new();
11431147
// On the paint for the layer, we don't need anti-aliasing on the fringes,
11441148
// since we are just blitting a texture. This saves a triangle strip for the stroke.
11451149
let layer_image_paint =
11461150
layer_image.as_paint_with_alpha(alpha_tint).with_anti_alias(false);
11471151

1148-
layer_path.rect(0., 0., layer_size.width as _, layer_size.height as _);
1149-
self.canvas.borrow_mut().fill_path(&layer_path, &layer_image_paint);
1152+
self.canvas.borrow_mut().save_with(|canvas| {
1153+
canvas.translate(layer_origin.x, layer_origin.y);
1154+
layer_path.rect(0., 0., layer_size.width as _, layer_size.height as _);
1155+
canvas.fill_path(&layer_path, &layer_image_paint);
1156+
});
11501157
}
11511158
RenderingResult::ContinueRenderingWithoutChildren
11521159
}

0 commit comments

Comments
 (0)