[webkit-dev] Issue Analysis:48290 [HTML Canvas globalCompositeOperation]
Hi All, I was going through the bug https://bugs.webkit.org/show_bug.cgi?id=48290, which passes is FF but fails in Safari/Winlauncher( r87771). As per http://www.w3.org/TR/2011/WD-2dcontext-20110525/#compositing, highlight is NOT a valid composite operation. but it is being mentioned in platform/graphics/GraphicsTypes.cpp in compositeOperatorNames[] that's why this issue is present. When I removed highlight from here also removed CompositeHighLight from enum CompositeOperator in GraphicsType.h this issue is resolved (I also had to comment out all other code using CompositeHighLight CSSValueHighlight ). However, the following comment was mentioned in GraphicsType.h (// Note: These constants exactly match the NSCompositeOperator constants of // AppKit on Mac OS X Tiger. If these ever change, we'll need to change the // Mac OS X Tiger platform code to map one to the other. ). So I am little unclear what is the purpose of this CompositeHighLight. Can someone please help me out here. Thanks Regards, Karthik ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Issue Analysis:48290 [HTML Canvas globalCompositeOperation]
On May 31, 2011, at 11:49 PM, Karthik wrote: However, the following comment was mentioned in GraphicsType.h (// Note: These constants exactly match the NSCompositeOperator constants of // AppKit on Mac OS X Tiger. If these ever change, we'll need to change the // Mac OS X Tiger platform code to map one to the other. ). So I am little unclear what is the purpose of this CompositeHighLight. We can remove that comment. It’s obsolete. I believe the “highlight” compositing mode used to be supported in canvas by WebKit on Mac, but hasn’t been for some time. There’s probably no harm in removing it now. The fact that the string is “not valid” in the specification isn’t what makes it OK to remove. Please remember that the canvas implementation in WebKit was the first and predates the specification by at least a year. But it’s almost certainly OK to remove this compositing mode because it hasn’t been implemented for quite a while. There’s still a slim chance that there is some content out there relying on “highlight” being a synonym for “source-over”, but it’s not likely. One side comment: I think the mapping from strings to the various graphics enums really belongs in the canvas code, not in the platform directory. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Issue Analysis:48290 [HTML Canvas globalCompositeOperation]
Hi Darin, Thanks for your detailed explanation. Initially we decided to remove the string as well the enum to fix this issue. But Karthik's patch (we have already submitted the patch in https://bugs.webkit.org/show_bug.cgi?id=48290) caused a Chromium build issue then we found that this enum is used in lot of other places. So, we were a little apprehensive about the side effect of this fix. Therefore, we submitted another patch where we are returning from the canvas code itself if the compositing mode is highlight. We are still waiting for the review comments. Similar issue (https://bugs.webkit.org/show_bug.cgi?id=48289 ) we have also seen for darker/CompositePlusDarker (not a valid compositing mode any more). Can we also remove this as well? Thanks, Rahaman On Wed, Jun 1, 2011 at 9:51 PM, Darin Adler da...@apple.com wrote: On May 31, 2011, at 11:49 PM, Karthik wrote: However, the following comment was mentioned in GraphicsType.h (// Note: These constants exactly match the NSCompositeOperator constants of // AppKit on Mac OS X Tiger. If these ever change, we'll need to change the // Mac OS X Tiger platform code to map one to the other. ). So I am little unclear what is the purpose of this CompositeHighLight. We can remove that comment. It’s obsolete. I believe the “highlight” compositing mode used to be supported in canvas by WebKit on Mac, but hasn’t been for some time. There’s probably no harm in removing it now. The fact that the string is “not valid” in the specification isn’t what makes it OK to remove. Please remember that the canvas implementation in WebKit was the first and predates the specification by at least a year. But it’s almost certainly OK to remove this compositing mode because it hasn’t been implemented for quite a while. There’s still a slim chance that there is some content out there relying on “highlight” being a synonym for “source-over”, but it’s not likely. One side comment: I think the mapping from strings to the various graphics enums really belongs in the canvas code, not in the platform directory. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Issue Analysis:48290 [HTML Canvas globalCompositeOperation]
On Jun 1, 2011, at 10:14 AM, Mustafizur Rahaman wrote: Therefore, we submitted another patch where we are returning from the canvas code itself if the compositing mode is highlight. That’s not a good idea. We should keep looking into the real fix; that kind of hack is unneeded here. Similar issue (https://bugs.webkit.org/show_bug.cgi?id=48289 ) we have also seen for darker/CompositePlusDarker (not a valid compositing mode any more). Can we also remove this as well? That’s different. That mode is implemented, so we’d have to be sure it wasn’t used. We can’t remove it just because it’s not in the specification without investigating what content depends on it. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
