Bug 68808

Summary: [Texmap][Qt] Refactor texture-upload to allow direct chunk update
Product: WebKit Reporter: Noam Rosenthal <noam>
Component: Layout and RenderingAssignee: Noam Rosenthal <noam>
Status: RESOLVED FIXED    
Severity: Normal CC: kenneth, kling, webkit.review.bot
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 56935, 60439    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Noam Rosenthal 2011-09-26 09:38:59 PDT
The current texture-mapper interface only allows for creating a BGRA32 buffer and painting into it.
But in WebKit2, we already have the pixels and want to update them directly into the texture without the extra copy.
Comment 1 Noam Rosenthal 2011-09-26 09:46:24 PDT
Created attachment 108680 [details]
Patch
Comment 2 Noam Rosenthal 2011-09-27 00:28:29 PDT
Created attachment 108798 [details]
Patch
Comment 3 Andreas Kling 2011-09-27 00:49:10 PDT
Comment on attachment 108798 [details]
Patch

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

> Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:543
> +static void* swizzleBGRAToRGBA(void* data, const IntSize& size)

This function should take something more specific than a void*.
Also, it has a return type but no return statements.

> Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:548
> +        uint* p = static_cast<uint*>(data) + y * (width * 4);

Is the number of bytes per row always guaranteed to be (width * 4)?

> Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:559
> +    void* swizzledBits = bits;

This appears to be unused.

> Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:586
> +         break;

Wrong indentation.
Comment 4 Noam Rosenthal 2011-09-27 05:00:29 PDT
Created attachment 108824 [details]
Patch
Comment 5 Noam Rosenthal 2011-09-27 05:02:47 PDT
> > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:586
> > +         break;
> 
> Wrong indentation.
Hmm... I fail to see the error of my ways here.
I believe the indentation is correct.
Comment 6 Andreas Kling 2011-09-27 05:06:06 PDT
(In reply to comment #5)
> > > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:586
> > > +         break;
> > 
> > Wrong indentation.
> Hmm... I fail to see the error of my ways here.
> I believe the indentation is correct.

It should line up with "glFormat = GL_BGR;" which it doesn't :)
Comment 7 Andreas Kling 2011-09-27 08:40:36 PDT
Comment on attachment 108824 [details]
Patch

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

r=me

> Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:585
> +        glFormat = GL_BGR;
> +#endif
> +         break;

Indentation police! Wiiiooowiiioooo!
Comment 8 WebKit Review Bot 2011-09-27 09:27:25 PDT
Comment on attachment 108824 [details]
Patch

Clearing flags on attachment: 108824

Committed r96118: <http://trac.webkit.org/changeset/96118>
Comment 9 WebKit Review Bot 2011-09-27 09:27:29 PDT
All reviewed patches have been landed.  Closing bug.