Bug 61740 - [Texmap][Qt] Enable TextureMapper by default
Summary: [Texmap][Qt] Enable TextureMapper by default
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Noam Rosenthal
URL:
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2011-05-30 11:12 PDT by Noam Rosenthal
Modified: 2011-06-07 17:57 PDT (History)
7 users (show)

See Also:


Attachments
Patch (985 bytes, patch)
2011-05-30 11:31 PDT, Noam Rosenthal
kling: review+
Details | Formatted Diff | Diff
Patch with a less awkward comment. (1.00 KB, patch)
2011-05-30 12:48 PDT, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (5.55 KB, patch)
2011-06-07 14:32 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-05-30 11:12:48 PDT
Since the 2.2 release has branched, now is the time to enable TextureMapper and find the remaining issues with it.
Comment 1 Noam Rosenthal 2011-05-30 11:31:54 PDT
Created attachment 95357 [details]
Patch

This has been discussed in the mailing list. 
It might have some effects on tests with platforms I can't test manually, so I'll get it through the commit queue once reviewed.
Comment 2 Andreas Kling 2011-05-30 12:40:36 PDT
Comment on attachment 95357 [details]
Patch

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

r=me, let's rock the boat!

> Source/WebCore/WebCore.pri:6
> +# Comment this to disable Texture Mapper.

This comment feels a little awkward.
Comment 3 Noam Rosenthal 2011-05-30 12:48:46 PDT
Created attachment 95362 [details]
Patch with a less awkward comment.
Comment 4 WebKit Commit Bot 2011-05-30 13:57:32 PDT
Comment on attachment 95362 [details]
Patch with a less awkward comment.

Clearing flags on attachment: 95362

Committed r87697: <http://trac.webkit.org/changeset/87697>
Comment 5 WebKit Commit Bot 2011-05-30 13:57:38 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Csaba Osztrogonác 2011-05-30 15:16:37 PDT
It broke Qt Windows build. Could you fix it ASAP?

It seems that OpenGL support is missing from the Qt 
on Windows bot. Andras, could you check it? If it
is true, some ifdef guards are missing from the patch.
Comment 7 Noam Rosenthal 2011-05-30 15:34:50 PDT
Fixed in r87700
Comment 8 Csaba Osztrogonác 2011-05-31 00:54:24 PDT
Symbian build is still broken. I have no idea why.
Comment 9 Simon Hausmann 2011-05-31 01:10:22 PDT
(In reply to comment #8)
> Symbian build is still broken. I have no idea why.

Looks like a problem with the OpenGL headers:

 compile    : Source\WebCore\platform\graphics\opengl\TextureMapperGL.cpp  	[armv5_urel.rvct4_0]
   "D:/w/qt-symbian-release/build/Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp", line 532: Error:  #20: identifier "GL_BGRA" is undefined
         GL_CMD(glTexSubImage2D(GL_TEXTURE_2D, 0, m_dirtyRect.x(), m_dirtyRect.y(), m_dirtyRect.width(), m_dirtyRect.height(), GL_BGRA, GL_UNSIGNED_BYTE, m_buffer->data()))
         ^

Should we build without OpenGL on Symbian perhaps?
Comment 10 Noam Rosenthal 2011-05-31 06:03:47 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > Symbian build is still broken. I have no idea why.
> 
> Looks like a problem with the OpenGL headers:
> 
>  compile    : Source\WebCore\platform\graphics\opengl\TextureMapperGL.cpp      [armv5_urel.rvct4_0]
>    "D:/w/qt-symbian-release/build/Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp", line 532: Error:  #20: identifier "GL_BGRA" is undefined
>          GL_CMD(glTexSubImage2D(GL_TEXTURE_2D, 0, m_dirtyRect.x(), m_dirtyRect.y(), m_dirtyRect.width(), m_dirtyRect.height(), GL_BGRA, GL_UNSIGNED_BYTE, m_buffer->data()))
>          ^
> 
> Should we build without OpenGL on Symbian perhaps?

This shouldn't even be built on symbian, unless unix|mac applies to Symbian somehow.
For now we should not enable TextureMapper for symbian, and eventually not build the OpenGL backend for Symbian.
Comment 11 Dihan Wickremasuriya 2011-05-31 08:17:39 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Symbian build is still broken. I have no idea why.
> > 
> > Looks like a problem with the OpenGL headers:
> > 
> >  compile    : Source\WebCore\platform\graphics\opengl\TextureMapperGL.cpp      [armv5_urel.rvct4_0]
> >    "D:/w/qt-symbian-release/build/Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp", line 532: Error:  #20: identifier "GL_BGRA" is undefined
> >          GL_CMD(glTexSubImage2D(GL_TEXTURE_2D, 0, m_dirtyRect.x(), m_dirtyRect.y(), m_dirtyRect.width(), m_dirtyRect.height(), GL_BGRA, GL_UNSIGNED_BYTE, m_buffer->data()))
> >          ^
> > 
> > Should we build without OpenGL on Symbian perhaps?
> 
> This shouldn't even be built on symbian, unless unix|mac applies to Symbian somehow.
> For now we should not enable TextureMapper for symbian, and eventually not build the OpenGL backend for Symbian.

Actually qmake considers Symbian to be a flavor of unix. To exclude the texmap flag for Symbian you'll need to guard it with a 'mac|unix:!symbian' scope.
Comment 12 Noam Rosenthal 2011-05-31 10:23:14 PDT
Should be fixed in r87739
Comment 13 Noam Rosenthal 2011-06-07 14:32:02 PDT
Created attachment 96298 [details]
Patch
Comment 14 Noam Rosenthal 2011-06-07 14:32:32 PDT
Reopening, so we can enable non-GL texture-mapper for Win/Symbian
Comment 15 Andreas Kling 2011-06-07 15:36:58 PDT
Comment on attachment 96298 [details]
Patch

rs=me
Comment 16 WebKit Review Bot 2011-06-07 17:57:29 PDT
Comment on attachment 96298 [details]
Patch

Clearing flags on attachment: 96298

Committed r88304: <http://trac.webkit.org/changeset/88304>
Comment 17 WebKit Review Bot 2011-06-07 17:57:35 PDT
All reviewed patches have been landed.  Closing bug.