Canvas has a proposal to allow blending in the compositing operator. The plaform code needs to be enhanced to support this. This bug will just extend the platform interface to allow this.
Created attachment 177847 [details] Patch
Comment on attachment 177847 [details] Patch Attachment 177847 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/15170261
Created attachment 177854 [details] Patch
Created attachment 177866 [details] Patch
Attachment 177866 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebKit/chromium/tests/DragImageTest.cpp:87: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 1 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 177866 [details] Patch Attachment 177866 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15173128
Comment on attachment 177866 [details] Patch Attachment 177866 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/15153648
Created attachment 177908 [details] Patch
Attachment 177908 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/html/HTMLCanvasElement.cpp'..." exit_code: 1 Source/WebKit/chromium/tests/ImageLayerChromiumTest.cpp:99: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebKit/chromium/tests/DragImageTest.cpp:87: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.
This patch updates interfaces so relevant calls also pass blend modes. In addition, the blend mode is stored in the graphics context.
Comment on attachment 177908 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177908&action=review > Source/WebCore/platform/graphics/GraphicsContext.h:409 > void setCompositeOperation(CompositeOperator); > + void setCompositeOperation(CompositeOperator, BlendMode); Does it make sense to combine both and use the default value BlendModeNormal like you did for setPlatformCompositeOperation ?
Created attachment 177949 [details] Patch
Attachment 177949 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebKit/chromium/tests/ImageLayerChromiumTest.cpp:99: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebKit/chromium/tests/DragImageTest.cpp:87: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 36 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 177908 [details] Patch Attachment 177908 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15154752
Comment on attachment 177949 [details] Patch Attachment 177949 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15181029
Comment on attachment 177949 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177949&action=review > Source/WebCore/platform/graphics/GraphicsContext.cpp:475 > +{ if (paintingDisabled() || !image) Missing a newline in here. > Source/WebCore/platform/graphics/GraphicsContext.cpp:548 > + drawImageBuffer(image, styleColorSpace, p, IntRect(0, 0, -1, -1), op, blendMode); I am not a fan of all of the methods in GraphicsContext named drawImageBuffer that chain together to finally end up at the one call with the FloatRect, FloatRect arguments. Can short-circuit all of these calls and go direct to the correct method? I'll probably take it on as a separate patch. > Source/WebCore/platform/graphics/GraphicsContext.cpp:553 > + drawImageBuffer(image, styleColorSpace, r, IntRect(0, 0, -1, -1), op, blendMode, useLowQualityScale); Ditto. > Source/WebCore/platform/graphics/GraphicsContext.cpp:558 > + drawImageBuffer(image, styleColorSpace, IntRect(dest, srcRect.size()), srcRect, op, blendMode); Ditto. > Source/WebCore/platform/graphics/GraphicsContext.h:409 > + void setCompositeOperation(CompositeOperator, BlendMode); Why not add BlendMode as a default parameter to the existing method? It would be functionally the same and avoid further bloat to this class. >> Source/WebKit/chromium/tests/DragImageTest.cpp:87 >> + WebCore::CompositeOperator, WebCore::BlendMode) > > Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Ignore this. >> Source/WebKit/chromium/tests/ImageLayerChromiumTest.cpp:99 >> + WebCore::CompositeOperator, WebCore::BlendMode) > > Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Ignore this.
Created attachment 178089 [details] Patch
Attachment 178089 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebKit/chromium/tests/ImageLayerChromiumTest.cpp:99: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebKit/chromium/tests/DragImageTest.cpp:87: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 37 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #11) > (From update of attachment 177908 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=177908&action=review > > > Source/WebCore/platform/graphics/GraphicsContext.h:409 > > void setCompositeOperation(CompositeOperator); > > + void setCompositeOperation(CompositeOperator, BlendMode); > > Does it make sense to combine both and use the default value BlendModeNormal like you did for setPlatformCompositeOperation ? I did not combine them because there are dependencies between the different WebKit libraries and I wanted to minimize the changes.
(In reply to comment #16) > (From update of attachment 177949 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=177949&action=review > > > Source/WebCore/platform/graphics/GraphicsContext.cpp:475 > > +{ if (paintingDisabled() || !image) > > Missing a newline in here. Fixed > > > Source/WebCore/platform/graphics/GraphicsContext.cpp:548 > > + drawImageBuffer(image, styleColorSpace, p, IntRect(0, 0, -1, -1), op, blendMode); > > I am not a fan of all of the methods in GraphicsContext named drawImageBuffer that chain together to finally end up at the one call with the FloatRect, FloatRect arguments. Can short-circuit all of these calls and go direct to the correct method? I'll probably take it on as a separate patch. I agree. It's messy. when you say "I'll take it on", do you mean you or me? > > > Source/WebCore/platform/graphics/GraphicsContext.cpp:553 > > + drawImageBuffer(image, styleColorSpace, r, IntRect(0, 0, -1, -1), op, blendMode, useLowQualityScale); > > Ditto. > > > Source/WebCore/platform/graphics/GraphicsContext.cpp:558 > > + drawImageBuffer(image, styleColorSpace, IntRect(dest, srcRect.size()), srcRect, op, blendMode); > > Ditto. > > > Source/WebCore/platform/graphics/GraphicsContext.h:409 > > + void setCompositeOperation(CompositeOperator, BlendMode); > > Why not add BlendMode as a default parameter to the existing method? It would be functionally the same and avoid further bloat to this class. > > >> Source/WebKit/chromium/tests/DragImageTest.cpp:87 > >> + WebCore::CompositeOperator, WebCore::BlendMode) > > > > Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] > > Ignore this. > > >> Source/WebKit/chromium/tests/ImageLayerChromiumTest.cpp:99 > >> + WebCore::CompositeOperator, WebCore::BlendMode) > > > > Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] > > Ignore this.
(In reply to comment #16) > (From update of attachment 177949 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=177949&action=review > > > Source/WebCore/platform/graphics/GraphicsContext.cpp:475 > > +{ if (paintingDisabled() || !image) > > Missing a newline in here. > > > Source/WebCore/platform/graphics/GraphicsContext.cpp:548 > > + drawImageBuffer(image, styleColorSpace, p, IntRect(0, 0, -1, -1), op, blendMode); > > I am not a fan of all of the methods in GraphicsContext named drawImageBuffer that chain together to finally end up at the one call with the FloatRect, FloatRect arguments. Can short-circuit all of these calls and go direct to the correct method? I'll probably take it on as a separate patch. > > > Source/WebCore/platform/graphics/GraphicsContext.cpp:553 > > + drawImageBuffer(image, styleColorSpace, r, IntRect(0, 0, -1, -1), op, blendMode, useLowQualityScale); > > Ditto. > > > Source/WebCore/platform/graphics/GraphicsContext.cpp:558 > > + drawImageBuffer(image, styleColorSpace, IntRect(dest, srcRect.size()), srcRect, op, blendMode); > > Ditto. > > > Source/WebCore/platform/graphics/GraphicsContext.h:409 > > + void setCompositeOperation(CompositeOperator, BlendMode); > > Why not add BlendMode as a default parameter to the existing method? It would be functionally the same and avoid further bloat to this class. Krit had the same remark. If I change this, there will be a link issues between the libraries. I can fix this in another patch. > > >> Source/WebKit/chromium/tests/DragImageTest.cpp:87 > >> + WebCore::CompositeOperator, WebCore::BlendMode) > > > > Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] > > Ignore this. > > >> Source/WebKit/chromium/tests/ImageLayerChromiumTest.cpp:99 > >> + WebCore::CompositeOperator, WebCore::BlendMode) > > > > Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] > > Ignore this.
Created attachment 178099 [details] Patch
Attachment 178099 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebKit/chromium/tests/ImageLayerChromiumTest.cpp:99: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebKit/chromium/tests/DragImageTest.cpp:87: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 37 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #20) > (In reply to comment #16) > > (From update of attachment 177949 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=177949&action=review > > > Source/WebCore/platform/graphics/GraphicsContext.cpp:548 > > > + drawImageBuffer(image, styleColorSpace, p, IntRect(0, 0, -1, -1), op, blendMode); > > > > I am not a fan of all of the methods in GraphicsContext named drawImageBuffer that chain together to finally end up at the one call with the FloatRect, FloatRect arguments. Can short-circuit all of these calls and go direct to the correct method? I'll probably take it on as a separate patch. > > I agree. It's messy. > when you say "I'll take it on", do you mean you or me? I mean myself. https://bugs.webkit.org/show_bug.cgi?id=104367
Comment on attachment 178099 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=178099&action=review > Source/WebCore/platform/graphics/GraphicsContext.cpp:710 > void GraphicsContext::setCompositeOperation(CompositeOperator compositeOperation) > { > + setCompositeOperation(compositeOperation, BlendModeNormal); > +} I don't get your point here actually. Which libraries do you mean? All files that include this functions can still use setCompositeOperation(compositeOperation)? Can you give an example please?
Created attachment 178242 [details] Patch
Attachment 178242 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebKit/chromium/tests/ImageLayerChromiumTest.cpp:99: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebKit/chromium/tests/DragImageTest.cpp:87: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 37 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #25) > (From update of attachment 178099 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=178099&action=review > > > Source/WebCore/platform/graphics/GraphicsContext.cpp:710 > > void GraphicsContext::setCompositeOperation(CompositeOperator compositeOperation) > > { > > + setCompositeOperation(compositeOperation, BlendModeNormal); > > +} > > I don't get your point here actually. Which libraries do you mean? All files that include this functions can still use setCompositeOperation(compositeOperation)? Can you give an example please? I changed the .def.in file that defines the exported functions. This fixed the problem.
Comment on attachment 178242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=178242&action=review Can you just upload a new patch with a better change log? > Source/WebCore/ChangeLog:8 > + Interface changes for the new blend modes Can you provide more details here? The changelog helps other people to understand what exaclty you did, w/o looking at the code.
Created attachment 178266 [details] Patch
Attachment 178266 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebKit/chromium/tests/ImageLayerChromiumTest.cpp:99: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebKit/chromium/tests/DragImageTest.cpp:87: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 37 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 178266 [details] Patch Clearing flags on attachment: 178266 Committed r136993: <http://trac.webkit.org/changeset/136993>
All reviewed patches have been landed. Closing bug.
Re-opened since this is blocked by bug 104415
Created attachment 178304 [details] Added missing exports file for mac
Attachment 178304 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebKit/chromium/tests/ImageLayerChromiumTest.cpp:99: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebKit/chromium/tests/DragImageTest.cpp:87: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 38 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 178304 [details] Added missing exports file for mac Lets try it agin. r=me.
Comment on attachment 178304 [details] Added missing exports file for mac Clearing flags on attachment: 178304 Committed r137011: <http://trac.webkit.org/changeset/137011>