Bug 35057 - Set viewport to canvas size upon context creation
Summary: Set viewport to canvas size upon context creation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Zhenyao Mo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-17 14:28 PST by Kenneth Russell
Modified: 2010-02-26 07:30 PST (History)
5 users (show)

See Also:


Attachments
patch (7.17 KB, patch)
2010-02-24 18:08 PST, Zhenyao Mo
no flags Details | Formatted Diff | Diff
revised patch: changed the log text according to Ken's advice (7.21 KB, patch)
2010-02-24 18:40 PST, Zhenyao Mo
levin: review-
Details | Formatted Diff | Diff
revised patch: added per change description in log files. (7.59 KB, patch)
2010-02-25 10:15 PST, Zhenyao Mo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.