Bug 117223

Summary: [CSS Blending] Updating the -webkit-background-blend-mode property dynamically does not trigger a redraw of the element
Product: WebKit Reporter: Mihai Tica <mitica>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, scraciun, WebkitBugTracker
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 108546    
Attachments:
Description Flags
Simplified test case
none
Fix without test
none
Patch for review
none
Updated test none

Mihai Tica
Reported 2013-06-04 22:39:33 PDT
Steps to reproduce this issue: 1. Load http://codepen.io/mihait/pen/uJBmF using a WebKit nightly version 2. Choose another blending mode from the select box. Note how the element with the background blend mode does not change it's appearance. Performing an action that triggers a redraw, such a scroll triggers the change.
Attachments
Simplified test case (1.21 KB, text/html)
2013-06-12 01:39 PDT, Mihai Tica 		no flags
Details
Fix without test (1.05 KB, patch)
2013-06-12 17:19 PDT, Dirk Schulze 		no flags
DetailsFormatted DiffDiff
Patch for review (6.37 KB, patch)
2013-06-12 22:54 PDT, Mihai Tica 		no flags
DetailsFormatted DiffDiff
Updated test (4.69 KB, patch)
2013-06-13 10:14 PDT, Mihai Tica 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mihai Tica
Comment 1 2013-06-12 01:39:40 PDT
Created attachment 204419 [details] Simplified test case This issue is only visible if the -webkit-background-blend mode is set from CSS and then updated from javascript
Dirk Schulze
Comment 2 2013-06-12 17:19:13 PDT
Created attachment 204548 [details] Fix without test The following patch should fix it. Not the sets should be compared (they are just used to fill all long hand properties), but the actual values.
Mihai Tica
Comment 3 2013-06-12 22:54:31 PDT
Created attachment 204554 [details] Patch for review Adding patch for review, including layout test. Thanks Dirk for looking into this
Dirk Schulze
Comment 4 2013-06-13 07:50:22 PDT
Comment on attachment 204554 [details] Patch for review View in context: https://bugs.webkit.org/attachment.cgi?id=204554&action=review > LayoutTests/css3/compositing/background-blend-mode-image-color-dynamic.html:29 > + for (var i = 0; i < blendModes.length; ++i) { > + var elem = document.createElement('div'); > + document.body.appendChild(elem); > + } Are you sure that this test really does the job of testing repaint? I would use the repaint logic from DRT and have a pixel result of the changed area. Then you don't need to test all blend modes, but just if the drawn content changed at all. We already have test to verify that the property value changed. You should find real repaint tests all over the place.
Mihai Tica
Comment 5 2013-06-13 10:14:35 PDT
Created attachment 204626 [details] Updated test
Mihai Tica
Comment 6 2013-06-13 10:15:06 PDT
(In reply to comment #4) > (From update of attachment 204554 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=204554&action=review > > > LayoutTests/css3/compositing/background-blend-mode-image-color-dynamic.html:29 > > + for (var i = 0; i < blendModes.length; ++i) { > > + var elem = document.createElement('div'); > > + document.body.appendChild(elem); > > + } > > Are you sure that this test really does the job of testing repaint? I would use the repaint logic from DRT and have a pixel result of the changed area. Then you don't need to test all blend modes, but just if the drawn content changed at all. We already have test to verify that the property value changed. You should find real repaint tests all over the place. Ok, it looks like my test simulated what was in fast/repaint, but we should probably use the already existing mechanism. Updating test.
Dirk Schulze
Comment 7 2013-06-13 10:57:57 PDT
Comment on attachment 204626 [details] Updated test View in context: https://bugs.webkit.org/attachment.cgi?id=204626&action=review LGTM. r=me. > LayoutTests/fast/repaint/background-blend-mode-image-color-dynamic-expected.html:15 > + <div style="-webkit-background-blend-mode: multiply, normal"></div> I guess this makes it easier for platforms that do not support blending to still pass. With other tests in place that test the same, it should be ok.
WebKit Commit Bot
Comment 8 2013-06-13 11:24:34 PDT
Comment on attachment 204626 [details] Updated test Clearing flags on attachment: 204626 Committed r151565: <http://trac.webkit.org/changeset/151565>
WebKit Commit Bot
Comment 9 2013-06-13 11:24:36 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.