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

Dongseong Hwang
Reported 2011-10-17 18:08:38 PDT
TextureMapperGL renders the strange border in http://www.webkit.org/blog-files/leaves/index.html
Attachments
Patch (3.56 KB, patch)
2011-10-17 18:24 PDT, Dongseong Hwang
no flags
patch (3.55 KB, patch)
2011-10-17 18:28 PDT, Dongseong Hwang
noam: review-
noam: commit-queue-
patch (3.54 KB, patch)
2011-10-17 19:08 PDT, Dongseong Hwang
noam: review+
noam: commit-queue-
patch (3.56 KB, patch)
2011-10-17 20:45 PDT, Dongseong Hwang
no flags
Dongseong Hwang
Comment 1 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.
WebKit Review Bot
Comment 2 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.
Dongseong Hwang
Comment 3 2011-10-17 18:28:47 PDT
Created attachment 111363 [details] patch Make coding style valid.
Noam Rosenthal
Comment 4 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)
Dongseong Hwang
Comment 5 2011-10-17 19:08:54 PDT
Created attachment 111371 [details] patch Thank you for proofreading.
Noam Rosenthal
Comment 6 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...
Dongseong Hwang
Comment 7 2011-10-17 20:09:53 PDT
I made mistake. :) Of course, I want you to review.
Noam Rosenthal
Comment 8 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.
Dongseong Hwang
Comment 9 2011-10-17 20:45:17 PDT
WebKit Review Bot
Comment 10 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>
WebKit Review Bot
Comment 11 2011-10-17 22:03:21 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.