Bug 70293 – [TexMap][QT] TexMapGL renders a strange one-pixel border of BitmapTexture.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Dongseong Hwang]

TextureMapperGL renders the strange border in http://www.webkit.org/blog-files/leaves/index.html

[Dongseong Hwang]

Created attachment 111362 [details]
Patch

TextureMapperGL renders the strange border in http://www.webkit.org/blog-files/leaves/index.html
It is because BitmapTexture is larger than original bitmap due to POT(Power of Two) and the rest space of BitmapTexture would be filled undefined values.

[WebKit Review Bot]

Attachment 111362 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:509:  PIXEL_SIZE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dongseong Hwang]

Created attachment 111363 [details]
patch

Make coding style valid.

[Noam Rosenthal]

Comment on attachment 111363 [details]
patch

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

Good catch!
Some nitpicks and we're done.

> Source/WebCore/ChangeLog:9
> +        It is because BitmapTexture is larger than original bitmap due to POT and the
> +        rest space of BitmapTexture would be filled undefined values.

Grammar:
Bug occurs because BitmapTexture is larger than the content due to rounding to NPOT, and its pixel data is never initialized.

> Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:507
> +static void texImage2DResourceSafe(size_t width, size_t height)

Don't like the name... how about resetTexturePixelData?

> Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:511
> +    if (width > 0 && height > 0) {

those are unsigned, so 
if (width && height)

[Dongseong Hwang]

Created attachment 111371 [details]
patch

Thank you for proofreading.

[Noam Rosenthal]

(In reply to comment #5)
> Created an attachment (id=111371) [details]
> patch
> 
> Thank you for proofreading.
You should r? it if you want me to review...

[Dongseong Hwang]

I made mistake. :)
Of course, I want you to review.

[Noam Rosenthal]

Comment on attachment 111371 [details]
patch

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

You can upload a new patch with r=me, cq? and those corrections :)

> Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:506
> +// Copy from GraphicsContext3D::texImage2DResourceSafe

Please put a period at the end of the sentence before committing.
I didn't realize it's the same function name as GraphicsContext3D - in that case you can rename it back.

[Dongseong Hwang]

Created attachment 111373 [details]
patch

[WebKit Review Bot]

Comment on attachment 111373 [details]
patch

Clearing flags on attachment: 111373

Committed r97718: <http://trac.webkit.org/changeset/97718>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.