[Webkit-unassigned] [Bug 48297] Fix LayoutTests/canvas/philip/tests/2d.composite.uncovered.image.destination-atop.html

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=48297





--- Comment #17 from Mustafizur Rahaman <mustaf.here at gmail.com>  2011-06-17 10:19:25 PST ---
(In reply to comment #16)
> (From update of attachment 97568 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=97568&action=review
> 
> > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1517
> > +    // to display transparency elsewhere, therefore adding the check for CompositeDestinationAtop
> 
> I would rather see a 'why' comment than a 'what' comment. Everybody can check against HTML5 that we are doing the right stuff here. However knowing *why* we need to add CompositeDestinationAtop but not CompositeSourceAtop is much more valuable to people reading the code and wondering why we have such a discrepancy.

I agree with you. Actually, I found that the comment was becoming too long to explain this in details, so thought of making it shorter. Anyway, will try to brief & precisely explain it.
> 
> > LayoutTests/platform/gtk/Skipped:-1104
> > -canvas/philip/tests/2d.composite.uncovered.image.source-out.html
> 
> Removing passing test from the Skipped list is great! However we prefer atomic patches. I would bet that source-{in|out} were passing without your change and thus are orthogonal to the current change. Could you split the patch in 2 and move the orthogonal Skipped changes to the other bugs (or correct me if I missed something).

If you see, we have removed the following test cases

-BUGWK39177 : canvas/philip/tests/2d.composite.uncovered.fill.destination-atop.html = TEXT
-BUGWK39177 : canvas/philip/tests/2d.composite.uncovered.pattern.destination-atop.html = TEXT
[The above test cases are related to our current changes of adding CompositeDestinationAtop in shouldDisplayTransparencyElsewhere()]

-BUGWK39177 : canvas/philip/tests/2d.composite.uncovered.image.destination-atop.html = TEXT
-BUGWK39177 : canvas/philip/tests/2d.composite.uncovered.image.destination-in.html = TEXT
-BUGWK39177 : canvas/philip/tests/2d.composite.uncovered.image.source-in.html = TEXT
-BUGWK39177 : canvas/philip/tests/2d.composite.uncovered.image.source-out.html = TEXT
[The above test cases are related to calling shouldDisplayTransparencyElsewhere() & displayTransparencyElsewhere() from drawImage(). So these test cases are also passing because of these changes, they were not passing before.]

Therefore, I don't see any need of splitting the changes. Let me know if I answered you.

Will update the patch with proper review comment.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.