Bug 70293

Summary: [TexMap][QT] TexMapGL renders a strange one-pixel border of BitmapTexture.
Product: WebKit Reporter: Dongseong Hwang <dongseong.hwang>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: noam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch
none
patch
noam: review-, noam: commit-queue-
patch
noam: review+, noam: commit-queue-
patch none

Description Dongseong Hwang 2011-10-17 18:08:38 PDT
TextureMapperGL renders the strange border in http://www.webkit.org/blog-files/leaves/index.html
Comment 1 Dongseong Hwang 2011-10-17 18:24:37 PDT
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.
Comment 2 WebKit Review Bot 2011-10-17 18:27:07 PDT
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.
Comment 3 Dongseong Hwang 2011-10-17 18:28:47 PDT
Created attachment 111363 [details]
patch

Make coding style valid.
Comment 4 Noam Rosenthal 2011-10-17 18:34:38 PDT
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)
Comment 5 Dongseong Hwang 2011-10-17 19:08:54 PDT
Created attachment 111371 [details]
patch

Thank you for proofreading.
Comment 6 Noam Rosenthal 2011-10-17 19:45:38 PDT
(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...
Comment 7 Dongseong Hwang 2011-10-17 20:09:53 PDT
I made mistake. :)
Of course, I want you to review.
Comment 8 Noam Rosenthal 2011-10-17 20:28:09 PDT
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.
Comment 9 Dongseong Hwang 2011-10-17 20:45:17 PDT
Created attachment 111373 [details]
patch
Comment 10 WebKit Review Bot 2011-10-17 22:03:17 PDT
Comment on attachment 111373 [details]
patch

Clearing flags on attachment: 111373

Committed r97718: <http://trac.webkit.org/changeset/97718>
Comment 11 WebKit Review Bot 2011-10-17 22:03:21 PDT
All reviewed patches have been landed.  Closing bug.