Bug 68808 - [Texmap][Qt] Refactor texture-upload to allow direct chunk update
Summary: [Texmap][Qt] Refactor texture-upload to allow direct chunk update
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Noam Rosenthal
URL:
Keywords: Qt
Depends on:
Blocks: 56935 60439
  Show dependency treegraph
 
Reported: 2011-09-26 09:38 PDT by Noam Rosenthal
Modified: 2011-09-27 09:27 PDT (History)
3 users (show)

See Also:


Attachments
Patch (10.19 KB, patch)
2011-09-26 09:46 PDT, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (10.23 KB, patch)
2011-09-27 00:28 PDT, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (10.22 KB, patch)
2011-09-27 05:00 PDT, Noam Rosenthal
no flags Details | Formatted Diff | Diff

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