Bug 64612

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:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none

Brian Salomon

Reported 2011-07-15 11:53:22 PDT

Must make skia's GL context current before drawing in GraphicsContext::clearRect

Attachments
Patch (1.34 KB, patch) 2011-07-15 11:54 PDT, Brian Salomon	no flags	Details Formatted Diff Diff
Patch (1.35 KB, patch) 2011-07-15 11:59 PDT, Brian Salomon	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Brian Salomon

Comment 1 2011-07-15 11:54:46 PDT

Created attachment 101017 [details] Patch

Brian Salomon

Comment 2 2011-07-15 11:59:53 PDT

Created attachment 101019 [details] Patch

Brian Salomon

Comment 3 2011-07-15 12:01:12 PDT

(In reply to comment #2) > Created an attachment (id=101019) [details] > Patch Second patch fixes the Changelog.

Stephen White

Comment 4 2011-07-15 12:05:20 PDT

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?

Brian Salomon

Comment 5 2011-07-15 12:08:29 PDT

(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.

WebKit Review Bot

Comment 6 2011-07-15 12:44:36 PDT

Comment on attachment 101019 [details] Patch Clearing flags on attachment: 101019 Committed r91093: <http://trac.webkit.org/changeset/91093>

WebKit Review Bot

Comment 7 2011-07-15 12:44:40 PDT

All reviewed patches have been landed. Closing bug.

Stephen White

Comment 8 2011-07-19 14:19:58 PDT

(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.

Brian Salomon

Comment 9 2011-07-19 14:24:57 PDT

(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?

Stephen White

Comment 10 2011-07-19 14:37:12 PDT

(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?

Brian Salomon

Comment 11 2011-07-19 14:41:01 PDT

(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.

Note You need to log in before you can comment on or make changes to this bug.