Summary: | [Texmap] Use glScissors for clipping in TextureMapperGL when possible | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Noam Rosenthal <noam> | ||||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | jturcotte, kenneth, menard, mrobinson, ostap73, webkit.review.bot, zoltan | ||||||||
Priority: | P2 | Keywords: | Qt | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Noam Rosenthal
2012-02-01 14:28:05 PST
Created attachment 125017 [details]
Patch
Comment on attachment 125017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125017&action=review This looks good, but does the patch need to enable the scissor test for non-Qt ports as well? > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:452 > + glEnable(GL_SCISSOR_TEST); Shouldn't this be enabled for non-Qt ports as well? > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:736 > + GLint viewport[4]; > + glGetIntegerv(GL_VIEWPORT, viewport); > + glScissor(rect.x(), viewport[3] - rect.maxY(), rect.width(), rect.height()); I assume this is for handling flipped-Y situations here? Nice! (In reply to comment #2) > (From update of attachment 125017 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=125017&action=review > > This looks good, but does the patch need to enable the scissor test for non-Qt ports as well? > > > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:452 > > + glEnable(GL_SCISSOR_TEST); > > Shouldn't this be enabled for non-Qt ports as well? Yes, I was not sure about whether other ports wanted to control it themselves, especially the part that disables the scissor test on exit. I don't mind putting this disable/enable code outside the #ifdef. > > > Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:736 > > + GLint viewport[4]; > > + glGetIntegerv(GL_VIEWPORT, viewport); > > + glScissor(rect.x(), viewport[3] - rect.maxY(), rect.width(), rect.height()); > > I assume this is for handling flipped-Y situations here? Nice! Well, we have to do this somewhere or nothing will appear on the screen :) (In reply to comment #3) > Yes, I was not sure about whether other ports wanted to control it themselves, especially the part that disables the scissor test on exit. I don't mind putting this disable/enable code outside the #ifdef. I think it could be more general by using glIsEnabled beforehand and then restoring the previous value. I don't think this would properly handle cases where the context already had a scissored region before calling into the texture mapper, but I'm not sure if that's an interesting usecase. Created attachment 125037 [details]
Patch
Added glIsEnabled in beginPainting, and made some of it cross-platform.
Created attachment 125049 [details]
Patch
Comment on attachment 125049 [details] Patch Clearing flags on attachment: 125049 Committed r106524: <http://trac.webkit.org/changeset/106524> All reviewed patches have been landed. Closing bug. |