Bug 89012

Summary: [Qt] Create an extra atlas when there is no available buffer space.
Product: WebKit Reporter: Alexis Menard (darktears) <menard>
Component: WebKit QtAssignee: Alexis Menard (darktears) <menard>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, jturcotte, noam, webkit.review.bot, zoltan
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch none

Description Alexis Menard (darktears) 2012-06-13 09:46:12 PDT
[Qt] Create an extra atlas when there no available buffer space.
Comment 1 Alexis Menard (darktears) 2012-06-13 09:48:08 PDT
Created attachment 147342 [details]
Patch
Comment 2 Jocelyn Turcotte 2012-06-13 09:56:04 PDT
Comment on attachment 147342 [details]
Patch

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

> Source/WebKit2/ChangeLog:3
> +        [Qt] Create an extra atlas when there no available buffer space.

...when there is no...

> Source/WebKit2/ChangeLog:10
> +        are updated. It can happen that there no available buffer space for
> +        the current atlas used. In that case we can create a new one. One

"for the current atlas."? "for the currently used atlas."?

Looks fine to me otherwise.
Comment 3 Jocelyn Turcotte 2012-06-13 09:56:34 PDT
I'll let No'am have a look if he's around.
Comment 4 Alexis Menard (darktears) 2012-06-13 10:26:46 PDT
Created attachment 147354 [details]
Patch
Comment 5 Noam Rosenthal 2012-06-13 10:40:51 PDT
Comment on attachment 147354 [details]
Patch

We should only enable this for desktop, with some settings flag, or a build flag. Otherwise we're making our scratch-buffer memory usage unpredictable.
Comment 6 Alexis Menard (darktears) 2012-06-18 06:25:41 PDT
(In reply to comment #5)
> (From update of attachment 147354 [details])
> We should only enable this for desktop, with some settings flag, or a build flag. Otherwise we're making our scratch-buffer memory usage unpredictable.

It fixes an horrible bug that could totally happen where the drawing surface is big, and not necessarily on a desktop platform. It's not like we see checkerboard or something like that, we simply don't repaint some tiles for a while, making the scrolling just plainly horrible and buggy.

What about a compile flag where you specify the scratch buffer memory value like how many time we will create an atlas of 2000.
Comment 7 Alexis Menard (darktears) 2012-06-18 06:26:24 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 147354 [details] [details])
> > We should only enable this for desktop, with some settings flag, or a build flag. Otherwise we're making our scratch-buffer memory usage unpredictable.
> 
> It fixes an horrible bug that could totally happen where the drawing surface is big, and not necessarily on a desktop platform. It's not like we see checkerboard or something like that, we simply don't repaint some tiles for a while, making the scrolling just plainly horrible and buggy.
> 
> What about a compile flag where you specify the scratch buffer memory value like how many time we will create an atlas of 2000.

Also having the default case to be buggy is not really an option.
Comment 8 Jocelyn Turcotte 2012-06-18 06:42:15 PDT
(In reply to comment #7)
> Also having the default case to be buggy is not really an option.

I agree, I think that it's much better if the default behavior on any platform is perfectly rendered.
If a platform wants to make sure that those 2MB of extra RAM are not allocated, the compile flag should be the exception and not the other way around.

Right now this causes tearing on the non-desktop view as well, during full-page renders like page loads, large document modifications, etc.
Comment 9 Noam Rosenthal 2012-06-19 01:58:24 PDT
Comment on attachment 147354 [details]
Patch

OK, I agree with the comments.
Comment 10 WebKit Review Bot 2012-06-19 04:43:55 PDT
Comment on attachment 147354 [details]
Patch

Clearing flags on attachment: 147354

Committed r120705: <http://trac.webkit.org/changeset/120705>
Comment 11 WebKit Review Bot 2012-06-19 04:44:00 PDT
All reviewed patches have been landed.  Closing bug.