Bug 78677 – [Qt][WK2] Clipping is broken

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Noam Rosenthal]

We don't apply the scene clipping for QtSceneGraph correctly, so the web-page appears to render in the wrong places, like behind the address bar.

[Noam Rosenthal]

Created attachment 127829 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 127829 [details]
Patch

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

Great that you fixed the issue!

> Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp:835
> +    for (int i = data().sharedGLData().clipStack.size() - 1; i >=0; --i) {

style... missing space. Also why do you need to intersect them using reverse order?

[Noam Rosenthal]

Created attachment 127856 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 127856 [details]
Patch for landing

Rejecting attachment 127856 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/11550288

[Noam Rosenthal]

Created attachment 127861 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 127861 [details]
Patch for landing

Rejecting attachment 127861 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/11552265

[Noam Rosenthal]

Created attachment 127863 [details]
Patch

[WebKit Review Bot]

Comment on attachment 127863 [details]
Patch

Clearing flags on attachment: 127863

Committed r108274: <http://trac.webkit.org/changeset/108274>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Noam Rosenthal]

Reopening, this caused an assert at the end of painting if there's any layer-clipping in the page.

[Noam Rosenthal]

process 4345 is executing new program: /home/abecsi/devel/git/webkit-git-svn/WebKitBuild/Debug/bin/QtWebProcess
ASSERTION FAILED: !data().sharedGLData().clipStack.isEmpty()
../../../../Source/WebCore/platform/graphics/opengl/TextureMapperGL.cpp(857) : bool WebCore::TextureMapperGL::endScissorClip()
1   0x7ffff561e6f8 /home/abecsi/devel/git/webkit-git-svn/WebKitBuild/Debug/lib/libQtWebKit.so.4(_ZN7WebCore15TextureMapperGL14endScissorClipEv+0x78) [0x7ffff561e6f8]
2   0x7ffff561ee0b /home/abecsi/devel/git/webkit-git-svn/WebKitBuild/Debug/lib/libQtWebKit.so.4(_ZN7WebCore15TextureMapperGL7endClipEv+0x1b) [0x7ffff561ee0b]
3   0x7ffff4694663 /home/abecsi/devel/git/webkit-git-svn/WebKitBuild/Debug/lib/libQtWebKit.so.4(_ZN6WebKit18LayerTreeHostProxy23paintToCurrentGLContextERKN7WebCore20TransformationMatrixEfRKNS1_9FloatRectE+0x2cf) [0x7ffff4694663]
4   0x7ffff45e7bbe /home/abecsi/devel/git/webkit-git-svn/WebKitBuild/Debug/lib/libQtWebKit.so.4(_ZN6WebKit20DrawingAreaProxyImpl23paintToCurrentGLContextERKN7WebCore20TransformationMatrixEfRKNS1_9FloatRectE+0x5c) [0x7ffff45e7bbe]
5   0x7ffff45d2ed9 /home/abecsi/devel/git/webkit-git-svn/WebKitBuild/Debug/lib/libQtWebKit.so.4(_ZN20QQuickWebPagePrivate23paintToCurrentGLContextEv+0x187) [0x7ffff45d2ed9]
6   0x7ffff45d32a2 /home/abecsi/devel/git/webkit-git-

[Noam Rosenthal]

Created attachment 128017 [details]
Patch to fix regression

[Noam Rosenthal]

Created attachment 128346 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 128346 [details]
Patch

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

> Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:205
> +    void initStencil();

initialize?

> Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:348
> +    if (data().sharedGLData().currentClipState.scissorBox.isEmpty())

does the current add any value to the name? clipState would be current, so I think it is a bit obvious, but maybe i am missing something

> Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:766
> +    // Increase stencilIndex and apply stencil testing testing.

testing testing?

[Simon Hausmann]

Comment on attachment 128346 [details]
Patch

LGTM modulo Kenneth's suggestions (testing testing is tested :)

[Noam Rosenthal]

Created attachment 128471 [details]
Patch

[WebKit Review Bot]

Comment on attachment 128471 [details]
Patch

Clearing flags on attachment: 128471

Committed r108696: <http://trac.webkit.org/changeset/108696>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.