Summary: | [SKIA] Must make skia's GL context current before drawing in GraphicsContext::clearRect | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brian Salomon <bsalomon> | ||||||
Component: | Platform | Assignee: | Brian Salomon <bsalomon> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | jamesr, senorblanco, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Brian Salomon
2011-07-15 11:53:22 PDT
Created attachment 101017 [details]
Patch
Created attachment 101019 [details]
Patch
(In reply to comment #2) > Created an attachment (id=101019) [details] > Patch Second patch fixes the Changelog. Comment on attachment 101019 [details]
Patch
Looks good.
compositing/shadows/shadow-drawing.html looks like it was passing after James's change. I'm guessing it's only exercised when run with --enable-accelerated-drawing?
(In reply to comment #4) > (From update of attachment 101019 [details]) > Looks good. > > compositing/shadows/shadow-drawing.html looks like it was passing after James's change. I'm guessing it's only exercised when run with --enable-accelerated-drawing? Oh, yes, that is true. Comment on attachment 101019 [details] Patch Clearing flags on attachment: 101019 Committed r91093: <http://trac.webkit.org/changeset/91093> All reviewed patches have been landed. Closing bug. (In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 101019 [details] [details]) > > Looks good. > > > > compositing/shadows/shadow-drawing.html looks like it was passing after James's change. I'm guessing it's only exercised when run with --enable-accelerated-drawing? > > Oh, yes, that is true. In the future, IWBN to have a test which exercises this case directly. (In reply to comment #8) > (In reply to comment #5) > > (In reply to comment #4) > > > (From update of attachment 101019 [details] [details] [details]) > > > Looks good. > > > > > > compositing/shadows/shadow-drawing.html looks like it was passing after James's change. I'm guessing it's only exercised when run with --enable-accelerated-drawing? > > > > Oh, yes, that is true. > > In the future, IWBN to have a test which exercises this case directly. There are a bunch of clearRect tests in canvas/philip/tests. I guess they didn't provoke the bug because we lucked into being in the right GL context. Maybe a test having a popup would force us to have two SGC3Ds? (In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #5) > > > (In reply to comment #4) > > > > (From update of attachment 101019 [details] [details] [details] [details]) > > > > Looks good. > > > > > > > > compositing/shadows/shadow-drawing.html looks like it was passing after James's change. I'm guessing it's only exercised when run with --enable-accelerated-drawing? > > > > > > Oh, yes, that is true. > > > > In the future, IWBN to have a test which exercises this case directly. > > There are a bunch of clearRect tests in canvas/philip/tests. I guess they didn't provoke the bug because we lucked into being in the right GL context. Maybe a test having a popup would force us to have two SGC3Ds? Hmm, that could be tricky in DRT. How about a test where the clear is the first call? (And clears to something other than transparent black). Would that leave the graphics context unset? (In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #8) > > > (In reply to comment #5) > > > > (In reply to comment #4) > > > > > (From update of attachment 101019 [details] [details] [details] [details] [details]) > > > > > Looks good. > > > > > > > > > > compositing/shadows/shadow-drawing.html looks like it was passing after James's change. I'm guessing it's only exercised when run with --enable-accelerated-drawing? > > > > > > > > Oh, yes, that is true. > > > > > > In the future, IWBN to have a test which exercises this case directly. > > > > There are a bunch of clearRect tests in canvas/philip/tests. I guess they didn't provoke the bug because we lucked into being in the right GL context. Maybe a test having a popup would force us to have two SGC3Ds? > > Hmm, that could be tricky in DRT. How about a test where the clear is the first call? (And clears to something other than transparent black). Would that leave the graphics context unset? I don't think so because PlatformContextSkia::setSharedGraphicsContext() call makes it current. I'm not really sure how it is getting unset in these failures, though. |