Bug 129305

Summary: [CSS Blending] Incomplete repainting when setting blend mode on an element
Product: WebKit Reporter: Mihai Tica <mitica>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, kondapallykalyan, krit, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 99119    
Attachments:
Description Flags
Repaint test
none
Patch V1
none
Fix compile errors
none
Screenshot of expected versus current behaviour none

Description Mihai Tica 2014-02-25 06:27:19 PST
Scenario:
parent with z-index and position set, child with no blend mode.
Set a blend mode to the child from javascript. Note that the child isn't correctly blended.
Attached a repaint test.

This is probably caused by a layerFragment which is incorrectly not painted.
Comment 1 Mihai Tica 2014-02-25 06:28:31 PST
Created attachment 225145 [details]
Repaint test

Repaint test showing the problem.
Comment 2 Mihai Tica 2014-02-25 09:18:34 PST
Created attachment 225154 [details]
Patch V1
Comment 3 Mihai Tica 2014-02-26 01:02:02 PST
Created attachment 225238 [details]
Fix compile errors
Comment 4 Dirk Schulze 2014-02-27 04:36:32 PST
Comment on attachment 225238 [details]
Fix compile errors

View in context: https://bugs.webkit.org/attachment.cgi?id=225238&action=review

> LayoutTests/ChangeLog:12
> +        * fast/repaint/blend-mode-changed-isolation-expected.txt: Added.
> +        * fast/repaint/blend-mode-changed-isolation.html: Added.

Could you say please how the test tests the correct behavior? Can it be seen by the repaint rects in the expectation file? If so, can you add a brief description please?
Comment 5 Mihai Tica 2014-02-27 06:07:29 PST
Created attachment 225362 [details]
Screenshot of expected versus current behaviour

This screenshot shows the current behaviour (upper part) versus the actual behaviour (lower part of the image).

This test has two divs: the yellow top one creates a stacking context, while the second blue div is placed on top of the first one.
When setting -webkit-mix-blend-mode on the blue div from script, it should only blend with the contents of the yellow div, since it's the parent stacking context.
This means that the part of the blue div that doesn't overlap the parent shouldn't blend with the body - it should stay blue.

The problem I've found was that the lower fragment of the yellow div wouldn't get repainted when setting blend-mode dynamically.
Since the yellow div has the m_isolatesBlending member set to true, it should fully paint, create a transparency layer, thus achieving isolation.

In other words, when m_isolatesBlending is set to true, the layer and its children should be fully painted.

What do you think of this approach? Do you see any other alternatives?
Comment 6 Dirk Schulze 2014-02-27 10:45:28 PST
(In reply to comment #5)
> Created an attachment (id=225362) [details]
> Screenshot of expected versus current behaviour
> 
> This screenshot shows the current behaviour (upper part) versus the actual behaviour (lower part of the image).
> 
> This test has two divs: the yellow top one creates a stacking context, while the second blue div is placed on top of the first one.
> When setting -webkit-mix-blend-mode on the blue div from script, it should only blend with the contents of the yellow div, since it's the parent stacking context.
> This means that the part of the blue div that doesn't overlap the parent shouldn't blend with the body - it should stay blue.
> 
> The problem I've found was that the lower fragment of the yellow div wouldn't get repainted when setting blend-mode dynamically.
> Since the yellow div has the m_isolatesBlending member set to true, it should fully paint, create a transparency layer, thus achieving isolation.
> 
> In other words, when m_isolatesBlending is set to true, the layer and its children should be fully painted.
> 
> What do you think of this approach? Do you see any other alternatives?

Sorry, wasn't attached to bug report. I meant that I didn't see from the expected result if the test passes or not. I would prefer a non-pixel test as you have. Is it possible from the expected file to see if the test passes? If yes, could you add a note in the test please?
Comment 7 Mihai Tica 2014-02-28 01:00:50 PST
(In reply to comment #6)
> (In reply to comment #5)
> > Created an attachment (id=225362) [details] [details]
> > Screenshot of expected versus current behaviour
> > 
> > This screenshot shows the current behaviour (upper part) versus the actual behaviour (lower part of the image).
> > 
> > This test has two divs: the yellow top one creates a stacking context, while the second blue div is placed on top of the first one.
> > When setting -webkit-mix-blend-mode on the blue div from script, it should only blend with the contents of the yellow div, since it's the parent stacking context.
> > This means that the part of the blue div that doesn't overlap the parent shouldn't blend with the body - it should stay blue.
> > 
> > The problem I've found was that the lower fragment of the yellow div wouldn't get repainted when setting blend-mode dynamically.
> > Since the yellow div has the m_isolatesBlending member set to true, it should fully paint, create a transparency layer, thus achieving isolation.
> > 
> > In other words, when m_isolatesBlending is set to true, the layer and its children should be fully painted.
> > 
> > What do you think of this approach? Do you see any other alternatives?
> 
> Sorry, wasn't attached to bug report. I meant that I didn't see from the expected result if the test passes or not. I would prefer a non-pixel test as you have. Is it possible from the expected file to see if the test passes? If yes, could you add a note in the test please?

While working on https://bugs.webkit.org/show_bug.cgi?id=129307, I figured out that this is not the correct way of handling this issue.

The solution I've posted implied painting a fragment that wasn't included in the dirty rect.
This is somehow a hack.

When isolation should be performed, the element representing the parent stacking context of a blended element should repaint, including its descendants.

I'm now working on posting this solution to the bug I've mentioned above.

Sorry for the noise
Comment 8 Mihai Tica 2014-02-28 01:01:40 PST

*** This bug has been marked as a duplicate of bug 129307 ***