Bug 77066

Summary: Implement new WEBGL compressed texture extensions
Product: WebKit Reporter: Gregg Tavares <gman>
Component: WebGLAssignee: Gregg Tavares <gman>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, cmarrin, gns, japhet, kbr, ojan, pnormand, rakuco, vestbo, webkit.review.bot, xan.lopez, zmo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Gregg Tavares 2012-01-25 18:18:34 PST
Upon approval of the new WebGL compressed texture extension specs, implement them in webkit

I'm happy to do this
Comment 1 Gregg Tavares 2012-02-03 18:40:25 PST
I have a CL for this but I'm running into a problem.

I wrote a layout test but the test is going to generate different results depending on how DRT is built. If it's using osmesa for example there is no texture compression AFAIK. If it's running on OSX there is probably always texture compression .

Thoughts?
Comment 2 Kenneth Russell 2012-02-06 16:43:36 PST
(In reply to comment #1)
> I have a CL for this but I'm running into a problem.
> 
> I wrote a layout test but the test is going to generate different results depending on how DRT is built. If it's using osmesa for example there is no texture compression AFAIK. If it's running on OSX there is probably always texture compression .
> 
> Thoughts?

Sorry for the delay replying -- was traveling and got behind on email.

Per offline discussion I think it's fine to split up the landing of the code and a layout test, since it's particularly hard to test this functionality across platforms. A WebGL conformance test would be great. Like the other conformance tests for extensions, it can exit early if the extension doesn't exist.
Comment 3 Gregg Tavares 2012-02-06 17:41:29 PST
Created attachment 125740 [details]
Patch
Comment 4 Kenneth Russell 2012-02-06 17:47:11 PST
*** Bug 77257 has been marked as a duplicate of this bug. ***
Comment 5 Gregg Tavares 2012-02-06 17:50:08 PST
Just fyi, originally I was planning add the new extension, then delete the old one. But that would have required a bunch of crazy workarounds. It would have also required adding/removing files from the projects which in the past has been painful.

Doing them both together prevents both issues.
Comment 6 Kenneth Russell 2012-02-06 17:56:18 PST
Comment on attachment 125740 [details]
Patch

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

Looks great. Couple of tiny typos. Could you rebaseline the patch? r=me

> Source/WebCore/ChangeLog:7
> +        texture extension and implemenets the first new

tiny typo: implements

> Source/WebCore/ChangeLog:10
> +        A test in in the WebGL conformance tests in

in in -> is in

> Source/WebCore/html/canvas/WebGLCompressedTextureS3TC.cpp:2
> + * Copyright (C) 2011 Google Inc. All rights reserved.

2012
Comment 7 Gregg Tavares 2012-02-07 12:45:03 PST
Created attachment 125901 [details]
Patch
Comment 8 Philippe Normand 2012-02-07 12:54:40 PST
Comment on attachment 125901 [details]
Patch

Attachment 125901 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11449060
Comment 9 Early Warning System Bot 2012-02-07 13:24:13 PST
Comment on attachment 125901 [details]
Patch

Attachment 125901 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11460059
Comment 10 Gregg Tavares 2012-02-07 14:14:50 PST
Created attachment 125926 [details]
Patch
Comment 11 Philippe Normand 2012-02-07 14:33:50 PST
Comment on attachment 125926 [details]
Patch

Attachment 125926 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11462096
Comment 12 Gregg Tavares 2012-02-07 14:54:32 PST
Created attachment 125936 [details]
Patch
Comment 13 Philippe Normand 2012-02-07 15:01:34 PST
Comment on attachment 125936 [details]
Patch

Attachment 125936 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11461100
Comment 14 Martin Robinson 2012-02-07 15:12:06 PST
Comment on attachment 125936 [details]
Patch

Sorry, it seems that the patch is also missing the new generated files. They should be placed in the list of files under webcore_built_sources. That how automake knows that the file should be compiled and linked.
Comment 15 Gregg Tavares 2012-02-07 15:22:03 PST
Created attachment 125943 [details]
Patch
Comment 16 Kenneth Russell 2012-02-08 10:01:36 PST
Comment on attachment 125943 [details]
Patch

Great! Martin, thanks for the help.
Comment 17 WebKit Review Bot 2012-02-08 11:14:11 PST
Comment on attachment 125943 [details]
Patch

Clearing flags on attachment: 125943

Committed r107107: <http://trac.webkit.org/changeset/107107>
Comment 18 WebKit Review Bot 2012-02-08 11:14:20 PST
All reviewed patches have been landed.  Closing bug.