Bug 73814 – [Qt] Remove redundant m_glWidget->makeCurrent() calls in GraphicsContext3DQt.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Dongseong Hwang]

GraphicsContext3DQt's all APIs call m_glWidget->makeCurrent().
It causes a performance hit, especially in the page that has multi webgl contexts.

[Dongseong Hwang]

Created attachment 117863 [details]
patch

[Noam Rosenthal]

Comment on attachment 117863 [details]
patch

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

Good patch, insufficient ChangeLog.

> Source/WebCore/ChangeLog:8
> +        Reviewed by NOBODY (OOPS!).

Insufficient Changelog. Please describe that you've moved all the calls into makeContextCurrent.

[Dongseong Hwang]

That's simple reason. I followed GraphicsContext3DOpenGL.cpp.

There are two options.
1. makeContextCurrent();
2. m_private->makeCurrentIfNeeded();

I chose 1 option.

Do you think I should describe above explanation in Changelog?

[Dongseong Hwang]

Comment on attachment 117863 [details]
patch

I want your response again, because there is no special reason.

[Noam Rosenthal]

(In reply to comment #3)
> That's simple reason. I followed GraphicsContext3DOpenGL.cpp.
> 
> There are two options.
> 1. makeContextCurrent();
> 2. m_private->makeCurrentIfNeeded();
> 
> I chose 1 option.
> 
> Do you think I should describe above explanation in Changelog?

Not really, just say something like "moved the redundant function calls to makeContextCurrent()" should be enough.

[Dongseong Hwang]

Created attachment 119714 [details]
Patch

[WebKit Review Bot]

Comment on attachment 119714 [details]
Patch

Attachment 119714 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10922016

New failing tests:
http/tests/inspector/resource-tree/resource-tree-document-url.html

[Noam Rosenthal]

Comment on attachment 119714 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Need a short description and bug URL (OOPS!)

Remove this line

> Source/WebCore/ChangeLog:14
> +        No new tests. (OOPS!)
> +

You say "No new functionality so no new tests"

[Dongseong Hwang]

Created attachment 119784 [details]
patch

I'm sorry for my carelessness.

[Noam Rosenthal]

(In reply to comment #9)
> Created an attachment (id=119784) [details]
> patch
> 
> I'm sorry for my carelessness.

It's no problem, we all forget this stuff once in a while :)

[WebKit Review Bot]

Comment on attachment 119784 [details]
patch

Clearing flags on attachment: 119784

Committed r103203: <http://trac.webkit.org/changeset/103203>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.