Skip to content
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

Support tiled clip images #2852

Closed
kvark opened this issue Jun 27, 2018 · 4 comments
Closed

Support tiled clip images #2852

kvark opened this issue Jun 27, 2018 · 4 comments

Comments

@kvark
Copy link
Member

@kvark kvark commented Jun 27, 2018

When we request image for a clip source that is an image, we never supply a tile:

                ClipSource::Image(ref mask) => {
                    resource_cache.request_image(
                        ImageRequest {
                            key: mask.image,
                            rendering: ImageRendering::Auto,
                            tile: None, // <---------- HERE
                        },
                        gpu_cache,
                    );
                }

This breaks the resource cache expectations that a tiled image would only be requested with specific tiles.
cc @nical @aosmond

@nical
Copy link
Collaborator

@nical nical commented Jun 28, 2018

Drawing the tiles into an an intermediate surface and using the latter as the clip source would make sense here.

gw3583 added a commit to gw3583/webrender that referenced this issue Oct 19, 2018
We don't currently support tiled blobs for clip masks (see
issue servo#2852). This patch detects that case, and skips applying
a clip mask in that case. It's not ideal, but it's better
than crashing inside the resource cache, which is the current
behavior.
@gw3583
Copy link
Collaborator

@gw3583 gw3583 commented Oct 19, 2018

I haven't thought about this too deeply, but I don't think we need an intermediate surface. I think to support this, we need to:

(1) Determine which of the image tiles overlap the clip mask bounds (we should be able to reuse some of the current tile visibility code for this).
(2) In the clip batcher, loop over each of those tiles and draw them into the clip mask at the appropriate location.

@nical
Copy link
Collaborator

@nical nical commented Oct 19, 2018

I think that the clip mask and what I called the intermediate surface are the same thing in my head (I haven't touched the masking code much so I'm a bit fuzzy about the terminology)

@emilio
Copy link
Member

@emilio emilio commented Oct 19, 2018

Will take a look at this.

@emilio emilio self-assigned this Oct 19, 2018
bors-servo added a commit that referenced this issue Oct 19, 2018
Skip clip masks with tiled blobs, instead of crashing.

We don't currently support tiled blobs for clip masks (see
issue #2852). This patch detects that case, and skips applying
a clip mask in that case. It's not ideal, but it's better
than crashing inside the resource cache, which is the current
behavior.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/3219)
<!-- Reviewable:end -->
@gw3583 gw3583 closed this in #3220 Oct 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.