Bug 61740

Summary: [Texmap][Qt] Enable TextureMapper by default
Product: WebKit Reporter: Noam Rosenthal <noam>
Component: Layout and RenderingAssignee: Noam Rosenthal <noam>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, commit-queue, dihan.wickremasuriya, hausmann, kenneth, ossy, webkit.review.bot
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
kling: review+
Patch with a less awkward comment.
none
Patch none

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.