Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upChange drop-shadows to be drawn via a single primitive. #2656
Conversation
Previously, drop-shadows were a Picture, with two brush image primitives, each referencing the picture (one for the content, one for the shadow with an offset). Although conceptually reasonable, this exposes some problems with the way the current prim_store visibility pass works. Specifically, we can end up processing the picture more than once. In general this is an inefficiency but doesn't affect correctness, but it breaks some assumptions once the content of the drop-shadow element is an item in the render task cache. In the future, this problem should disappear as we refactor how the visibility pass operates, but for now the simplest solution is to draw the drop-shadow with a single primitive. To make this work, there are a couple of hacks in this patch, which push an extra brush image primitive into the GPU cache data. The drop shadows still have an issue with the shadow disappearing if the content of the drop shadow is off-screen, however this is no worse than the existing code. Doing it this way actually makes it easier to fix this is the future too.
|
Try run looks good to me. |
I'd like to know more about this. Reviewed 5 of 5 files at r1. webrender/src/picture.rs, line 171 at r1 (raw file):
Comments from Reviewable |
|
@bors-servo r+ |
|
|
bors-servo
added a commit
that referenced
this pull request
Apr 13, 2018
Change drop-shadows to be drawn via a single primitive. Previously, drop-shadows were a Picture, with two brush image primitives, each referencing the picture (one for the content, one for the shadow with an offset). Although conceptually reasonable, this exposes some problems with the way the current prim_store visibility pass works. Specifically, we can end up processing the picture more than once. In general this is an inefficiency but doesn't affect correctness, but it breaks some assumptions once the content of the drop-shadow element is an item in the render task cache. In the future, this problem should disappear as we refactor how the visibility pass operates, but for now the simplest solution is to draw the drop-shadow with a single primitive. To make this work, there are a couple of hacks in this patch, which push an extra brush image primitive into the GPU cache data. The drop shadows still have an issue with the shadow disappearing if the content of the drop shadow is off-screen, however this is no worse than the existing code. Doing it this way actually makes it easier to fix this is the future too. <!-- 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/2656) <!-- Reviewable:end -->
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
glennw commentedApr 13, 2018
•
edited by larsbergstrom
Previously, drop-shadows were a Picture, with two brush image
primitives, each referencing the picture (one for the content,
one for the shadow with an offset).
Although conceptually reasonable, this exposes some problems
with the way the current prim_store visibility pass works.
Specifically, we can end up processing the picture more than
once. In general this is an inefficiency but doesn't affect
correctness, but it breaks some assumptions once the content
of the drop-shadow element is an item in the render task cache.
In the future, this problem should disappear as we refactor
how the visibility pass operates, but for now the simplest solution
is to draw the drop-shadow with a single primitive. To make this
work, there are a couple of hacks in this patch, which
push an extra brush image primitive into the GPU cache data.
The drop shadows still have an issue with the shadow disappearing
if the content of the drop shadow is off-screen, however this is
no worse than the existing code. Doing it this way actually makes
it easier to fix this is the future too.
This change is