Bug 97472 – accelerated compositing does not work with ati driver

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[arno.]

Created attachment 165419 [details]
testcase

Hi,
when running gtklauncher with --enable-accelerated-compositing to display a page which needs accelerated composition, the page is all messed up. It looks like some parts contain random data (for example, I sometimes see parts of other application windows).
I'm using ubuntu with ati driver (catalyst). I've just switched to mesa driver, and I now don't reproduce the problem.

I'm currently setting it to gtk component, but I don't known if it affects other platforms.

[arno.]

Created attachment 165420 [details]
screenshot with --enable-accelerated-compositing and catalyst driver

[arno.]

Actually, not all the page is messed up: only the 100 left pixels, and the 100 top pixels. It looks like it comes from the coordinates of m_parentWindow of RedirectedXCompositeWindow
If I call XCreateWindow with x=-50 and y=-50, only the 50 left and top pixels are messed up.
It looks like the part of m_window which is at negative coordinates will not be displayed.

[arno.]

Created attachment 168062 [details]
Patch

patch proposal: place m_parentWindow on the right side of the screen

[WebKit Review Bot]

Attachment 168062 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/platform/gtk/RedirectedXCompositeWindow.cpp:137:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/platform/gtk/RedirectedXCompositeWindow.cpp:138:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Martin Robinson]

Comment on attachment 168062 [details]
Patch

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

> Source/WebCore/platform/gtk/RedirectedXCompositeWindow.cpp:131
> +    // some drivers (fglrx) do not display the part of the window at negative
> +    // positions. So, to put the window at a hidden place, we put it at a
> +    // positing higher than screen dimensions.
> +    int screenWidth = WidthOfScreen(screen);
> +    int screenHeight = HeightOfScreen(screen);
> +    int windowX = 0; 
> +    int windowY = 0;
> +    if (screenWidth == INT_MAX) {
> +        if (screenHeight == INT_MAX) {
> +            // FIXME: if screen dimensions are maximal in both directions, and
> +            // we have a faulty driver, we will get a 1px border on bottom and
> +            // on left of the window which will not be drawn.
> +            windowX = -1;
> +            windowY = -1;
> +        } else
> +            windowY = screenHeight + 1;
> +    } else
> +        windowX = screenWidth + 1;

Could this be rewritten as:
int windowX = WidthOfScreen(screen);
int windowY = HeightOfScreen(screen);

or is it likely that WidthOfScreen or HeightOfScreen will return INT_MAX? That's a really big screen. It seems the worst case is that we'd have some rollover.

>> Source/WebCore/platform/gtk/RedirectedXCompositeWindow.cpp:137
>> +                                   RootWindowOfScreen(screen),
> 
> Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]

Please fix this section. The new style is that we should line up the variables.

[Martin Robinson]

(In reply to comment #5)

> Could this be rewritten as:
> int windowX = WidthOfScreen(screen);
> int windowY = HeightOfScreen(screen);

Sorry, I meant 

int windowX = WidthOfScreen(screen) + 1;
int windowY = HeightOfScreen(screen) + 1;

If so, you probably don't even need them to be stored in variables.

[arno.]

Created attachment 168066 [details]
Patch

updated patch

[WebKit Review Bot]

Comment on attachment 168066 [details]
Patch

Rejecting attachment 168066 [details] from commit-queue.

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

Last 500 characters of output:
mit-queue/Source/WebKit/chromium/third_party/skia/src --revision 5871 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
48>At revision 5871.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

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

[arno.]

Created attachment 168243 [details]
OOPS, I did not write the changelog. Here is the patch with an edited ChangeLog

updated patch

[WebKit Review Bot]

Comment on attachment 168243 [details]
OOPS, I did not write the changelog. Here is the patch with an edited ChangeLog

Clearing flags on attachment: 168243

Committed r131076: <http://trac.webkit.org/changeset/131076>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.