Bug 35057

Summary: Set viewport to canvas size upon context creation
Product: WebKit Reporter: Kenneth Russell <kbr>
Component: WebGLAssignee: Zhenyao Mo <zmo>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarrin, commit-queue, giles, kbr, oliver
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
none
revised patch: changed the log text according to Ken's advice
levin: review-
revised patch: added per change description in log files. none

Description Kenneth Russell 2010-02-17 14:28:12 PST
Per the latest draft of the WebGL specification, the OpenGL viewport must be set to (0, 0, canvas.width, canvas.height) upon context creation. This is a change in behavior compared to the recently incorporated fix https://bugs.webkit.org/show_bug.cgi?id=34766 .
Comment 1 Zhenyao Mo 2010-02-24 18:08:41 PST
Created attachment 49455 [details]
patch
Comment 2 Kenneth Russell 2010-02-24 18:31:41 PST
Looks good to me, though I'm not sure the text

Test: fast/canvas/webgl/gl-get-calls.html

in WebCore/ChangeLog is appropriate, since it matches the format prepare-ChangeLog uses when a new test is added. Perhaps say instead "Covered by existing tests, in particular fast/canvas/webgl/gl-get-calls.html".
Comment 3 Zhenyao Mo 2010-02-24 18:40:15 PST
Created attachment 49459 [details]
revised patch: changed the log text according to Ken's advice
Comment 4 David Levin 2010-02-25 08:57:18 PST
Comment on attachment 49459 [details]
revised patch: changed the log text according to Ken's advice

> Index: WebCore/ChangeLog
> +2010-02-24  Zhenyao Mo  <zmo@google.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Set viewport to canvas size upon context creation
> +        https://bugs.webkit.org/show_bug.cgi?id=35057
> +
> +        Covered by existing tests, in particular fast/canvas/webgl/gl-get-calls.html
> +
> +        * html/canvas/WebGLRenderingContext.cpp:
> +        (WebCore::WebGLRenderingContext::WebGLRenderingContext):

Ideally there are per file comments explaining the change. For example,

    (WebCore::WebGLRenderingContext::WebGLRenderingContext): Set the viewport siz


> Index: LayoutTests/ChangeLog
> +2010-02-24  Zhenyao Mo  <zmo@google.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Need a short description and bug URL (OOPS!)

Do you see what I see? :)



> +
> +        * fast/canvas/webgl/bug-32888.html:

Per file comments would be nice. For example "Removed call to set the viewport size since this is done when the context is created."

(You may say "Ditto." if the comment for a file is the same as the last comment.

> +        * fast/canvas/webgl/gl-get-calls-expected.txt:

A comment for this file would have a *very short* explanation of why the result changed.

> +        * fast/canvas/webgl/gl-get-calls.html:
> +        * fast/canvas/webgl/tex-image-and-sub-image-2d-with-image.html:
> +        * fast/canvas/webgl/tex-sub-image-2d.html:
> +        * fast/canvas/webgl/texImage2DImageDataTest.html:
> +        * fast/canvas/webgl/triangle.html:
> +        * fast/canvas/webgl/viewport-unchanged-upon-resize.html:
> +
Comment 5 Zhenyao Mo 2010-02-25 10:15:39 PST
Created attachment 49507 [details]
revised patch: added per change description in log files.
Comment 6 WebKit Commit Bot 2010-02-26 07:30:36 PST
Comment on attachment 49507 [details]
revised patch: added per change description in log files. 

Clearing flags on attachment: 49507

Committed r55282: <http://trac.webkit.org/changeset/55282>
Comment 7 WebKit Commit Bot 2010-02-26 07:30:41 PST
All reviewed patches have been landed.  Closing bug.