Bug 53568 - Fix pink video bug with gpu-acceleration enabled
Summary: Fix pink video bug with gpu-acceleration enabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Victoria Kirst
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-01 19:54 PST by Victoria Kirst
Modified: 2011-02-03 22:47 PST (History)
4 users (show)

See Also:

Attachments
Patch (6.22 KB, patch)
2011-02-01 19:56 PST, Victoria Kirst 		no flags Details | Formatted Diff | Diff
Patch (6.77 KB, patch)
2011-02-03 12:47 PST, Victoria Kirst 		no flags Details | Formatted Diff | Diff
Patch (6.24 KB, patch)
2011-02-03 12:53 PST, Victoria Kirst 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Victoria Kirst 2011-02-01 19:54:51 PST 
Fix pink video bug with gpu-acceleration enabled
Comment 1 Victoria Kirst 2011-02-01 19:56:34 PST 
Created attachment 80876 [details]
Patch
Comment 2 Kenneth Russell 2011-02-03 11:16:14 PST 
Comment on attachment 80876 [details]
Patch

We really should figure out a more general solution to this problem. Can we switch the locale of the GPU process to something neutral?

This workaround seems OK to me, but it's going to be fragile.
Comment 3 Victoria Kirst 2011-02-03 11:48:56 PST 
Thanks Ken!

I agree, this is a weak solution. Evan Martin recently uploaded a patch to force LC_NUMERIC to C for all non-browser processes, which should also theoretically fix this. (http://codereview.chromium.org/6347045) But since that patch is a bit scary and should have some baking time before we commit to it, we thought it'd be best to upload this in interim to merge into m10.
Comment 4 James Robinson 2011-02-03 12:41:07 PST 
Comment on attachment 80876 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=80876&action=review

> Source/WebCore/platform/graphics/chromium/VideoLayerChromium.cpp:108
> +        "  gl_FragColor = vec4(rgb, float(1) * alpha;         \n"

these parens are mismatched (pointed out by someone on the crbug)
Comment 5 Victoria Kirst 2011-02-03 12:47:42 PST 
Created attachment 81101 [details]
Patch
Comment 6 Victoria Kirst 2011-02-03 12:48:45 PST 
Comment on attachment 81101 [details]
Patch

Oops, argh, screwed up change log...
Comment 7 Victoria Kirst 2011-02-03 12:53:12 PST 
Created attachment 81102 [details]
Patch
Comment 8 Victoria Kirst 2011-02-03 12:56:04 PST 
> these parens are mismatched (pointed out by someone on the crbug)

Yikes, thanks for the catch, jamesr/crbug guy!!
Comment 9 Vangelis Kokkevis 2011-02-03 14:08:40 PST 
out of curiosity, would replacing 0.5 with (float)1/2 work?  It would touch less code...
Comment 10 WebKit Commit Bot 2011-02-03 22:46:59 PST 
Comment on attachment 81102 [details]
Patch

Clearing flags on attachment: 81102

Committed r77608: <http://trac.webkit.org/changeset/77608>
Comment 11 WebKit Commit Bot 2011-02-03 22:47:03 PST 
All reviewed patches have been landed.  Closing bug.