Skip to content

/ webrender

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip clip masks with tiled blobs, instead of crashing. #3219

Merged
merged 2 commits into from Oct 19, 2018
+57 −37
Merged

Skip clip masks with tiled blobs, instead of crashing. #3219

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4dc6618
Skip clip masks with tiled blobs, instead of crashing.
gw3583 Oct 19, 2018
207cfec
Add a comment for the boolean member in ClipItem::ImageMask.
emilio Oct 19, 2018
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

39 webrender/src/batch.rs
View file
Open in desktop
@@ -2018,25 +2018,30 @@ impl ClipBatcher {
let gpu_address = gpu_cache.get_address(&clip_node.gpu_cache_handle);

match clip_node.item {
ClipItem::Image(ref mask) => {
if let Ok(cache_item) = resource_cache.get_cached_image(
ImageRequest {
key: mask.image,
rendering: ImageRendering::Auto,
tile: None,
ClipItem::Image(ref mask, is_valid) => {
if is_valid {
if let Ok(cache_item) = resource_cache.get_cached_image(
ImageRequest {
key: mask.image,
rendering: ImageRendering::Auto,
tile: None,
}
) {
self.images
.entry(cache_item.texture_id)
.or_insert(Vec::new())
.push(ClipMaskInstance {
clip_data_address: gpu_address,
resource_address: gpu_cache.get_address(&cache_item.uv_rect_handle),
..instance
});
} else {
warn!("Warnings: skip a image mask");
debug!("Key:{:?} Rect::{:?}", mask.image, mask.rect);
continue;
}
) {
self.images
.entry(cache_item.texture_id)
.or_insert(Vec::new())
.push(ClipMaskInstance {
clip_data_address: gpu_address,
resource_address: gpu_cache.get_address(&cache_item.uv_rect_handle),
..instance
});
} else {
warn!("Warnings: skip a image mask");
debug!("Key:{:?} Rect::{:?}", mask.image, mask.rect);
warn!("Warnings: clip masks that are tiled blobs are not yet supported (#2852)");
continue;
}
}
53 webrender/src/clip.rs
View file
Open in desktop
@@ -145,11 +145,14 @@ impl From<ClipItemKey> for ClipNode {
)
}
ClipItemKey::ImageMask(rect, image, repeat) => {
ClipItem::Image(ImageMask {
image,
rect: LayoutRect::from_au(rect),
repeat,
})
ClipItem::Image(
ImageMask {
image,
rect: LayoutRect::from_au(rect),
repeat,
},
true,
)
}
ClipItemKey::BoxShadow(shadow_rect, shadow_radius, prim_shadow_rect, blur_radius, clip_mode) => {
ClipItem::new_box_shadow(
@@ -263,7 +266,7 @@ impl ClipNode {
) {
if let Some(mut request) = gpu_cache.request(&mut self.gpu_cache_handle) {
match self.item {
ClipItem::Image(ref mask) => {
ClipItem::Image(ref mask, ..) => {
let data = ImageMaskData { local_rect: mask.rect };
data.write_gpu_blocks(request);
}
@@ -294,15 +297,25 @@ impl ClipNode {
}

match self.item {
ClipItem::Image(ref mask) => {
resource_cache.request_image(
ImageRequest {
key: mask.image,
rendering: ImageRendering::Auto,
tile: None,
},
gpu_cache,
);
ClipItem::Image(ref mask, ref mut is_valid) => {
if let Some(properties) = resource_cache.get_image_properties(mask.image) {
// Clip masks with tiled blob images are not currently supported.
// This results in them being ignored, which is not ideal, but
// is better than crashing in resource cache!
// See https://github.com/servo/webrender/issues/2852.
*is_valid = properties.tiling.is_none();
}

if *is_valid {
resource_cache.request_image(
ImageRequest {
key: mask.image,
rendering: ImageRendering::Auto,
tile: None,
},
gpu_cache,
);
}
}
ClipItem::BoxShadow(ref mut info) => {
// Quote from https://drafts.csswg.org/css-backgrounds-3/#shadow-blur
@@ -760,7 +773,9 @@ impl ClipItemKey {
pub enum ClipItem {
Rectangle(LayoutRect, ClipMode),
RoundedRectangle(LayoutRect, BorderRadius, ClipMode),
Image(ImageMask),
/// The boolean below is a crash workaround for #2852, will be true unless
/// the mask is a tiled blob.
Image(ImageMask, bool),

This comment has been minimized.

Sign in to view
@nical

nical Oct 19, 2018

Collaborator

Please make the boolean a named field or add a comment, It takes a bit of looking around to find what it is for (batch.rs is the only place where the meaning is explicit, and it's not where I'd look first),
BoxShadow(BoxShadowClipSource),
}

@@ -873,8 +888,8 @@ impl ClipItem {
ClipItem::Rectangle(_, ClipMode::ClipOut) => None,
ClipItem::RoundedRectangle(clip_rect, _, ClipMode::Clip) => Some(clip_rect),
ClipItem::RoundedRectangle(_, _, ClipMode::ClipOut) => None,
ClipItem::Image(ref mask) if mask.repeat => None,
ClipItem::Image(ref mask) => Some(mask.rect),
ClipItem::Image(ref mask, ..) if mask.repeat => None,
ClipItem::Image(ref mask, ..) => Some(mask.rect),
ClipItem::BoxShadow(..) => None,
}
}
@@ -1006,7 +1021,7 @@ impl ClipItem {
}
}
}
ClipItem::Image(ref mask) => {
ClipItem::Image(ref mask, ..) => {
if mask.repeat {
ClipResult::Partial
} else {
2 webrender/src/hit_test.rs
View file
Open in desktop
@@ -39,7 +39,7 @@ impl HitTestClipNode {
ClipItem::Rectangle(ref rect, mode) => HitTestRegion::Rectangle(*rect, mode),
ClipItem::RoundedRectangle(ref rect, ref radii, ref mode) =>
HitTestRegion::RoundedRectangle(*rect, *radii, *mode),
ClipItem::Image(ref mask) => HitTestRegion::Rectangle(mask.rect, ClipMode::Clip),
ClipItem::Image(ref mask, ..) => HitTestRegion::Rectangle(mask.rect, ClipMode::Clip),
ClipItem::BoxShadow(_) => HitTestRegion::Invalid,
};

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.