search

servo / webrender

A GPU-based renderer for the web
https://doc.servo.org/webrender/
Mozilla Public License 2.0
3.12k stars 275 forks source link

More mix-blend modes issue with Servo (since c4b67b55) #3594

Open paulrouget opened 5 years ago

paulrouget commented 5 years ago

Since c4b67b55af68ab18c497861cd12b65a0107fd579 some mix blend mode tests do not pass anymore. For example: tests/wpt/web-platform-tests/css/compositing/compositing_simple_div.html

paulrouget commented 5 years ago

/cc @kvark @mattwoodrow

paulrouget commented 5 years ago

Is there anything special that should be done to reproduce the gecko behavior (according to the commit message: "Remove special handling for mix-blend modes on the root WebRender stacking context, since Gecko handles this already")

gw3583 commented 5 years ago

Could you post the reftest differences as screenshots? It's not totally clear to me what Gecko does differently here.

paulrouget commented 5 years ago 
<div style="background-color:rgb(255,0,255); mix-blend-mode:difference; width:100px;height:100px"></div>

Before WR update, we would get: after

After we get: before

mattwoodrow commented 5 years ago

The current code detects when we have mix-blend-mode on a stacking context, and makes sure that the parent stacking context is isolated (creates a temporary surface) so that we only blend with our siblings, not ancestors.

Previously, we had code to detect when the parent stacking context was the root stacking context, and skipped the isolation. This is because WebRender also has an implicit background color outside of the root stacking context, and we wanted to blend with that.

Gecko never wants that, and always inserts an explicit 'isolation' stacking context around mix-blend-mode, so can never hit that. The mentioned PR removed it for simplicity.

I don't really understand the failure here though, since the change is from not blending with anything (result is the same as the source color), to blending with a white background (changes to green). That sounds like we're isolating less, but the change should have only increased the set of situations where we isolate.

gw3583 commented 5 years ago

I'm also a bit confused, for the same reasons as Matt above.

Adding to my confusion - isn't it the case that the after result is the correct rendering? The div has magenta color, with difference mix-blend-mode. So the result should be the inverse color of magenta, which is green. I might be overlooking something simple - am I right that green should be the expected output for this test case?

paulrouget commented 5 years ago

Sorry, I mixed up the 2 screenshots.

This is what happens:

This is rendered correctly (green):

<body style="background:white">
<div style="background-color:rgb(255,0,255); mix-blend-mode:difference; width:100px;height:100px"></div>
</body>

This is not (pink):

<body>
<div style="background-color:rgb(255,0,255); mix-blend-mode:difference; width:100px;height:100px"></div>
</body>