Bug 78243 – [Qt] Be smarter with tile usages during tiling

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Kenneth Rohde Christiansen]

SSIA

[Kenneth Rohde Christiansen]

Created attachment 126310 [details]
Patch

[WebKit Review Bot]

Attachment 126310 [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/graphics/TiledBackingStore.h:97:  The parameter name "rect" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 4 files


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

[Simon Hausmann]

Comment on attachment 126310 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [Qt] Be smarted with tile usages during tiling

smarted -> smarter

> Source/WebCore/ChangeLog:13
> +        simple intersecting our areas with the contentRect, but moving it

simple -> simply

[Simon Hausmann]

Apart from my nitpicks and the style issue this looks good to me. But it would be even better if Jocelyn could peek it it, too :)

[Jocelyn Turcotte]

Comment on attachment 126310 [details]
Patch

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

The logic looks pretty good to me.

> Source/WebCore/ChangeLog:8
> +        The keep rect used for what existing tiles to keep around, is now a

Maybe add a verb instead of "for", "used to chose" or "used to know".

> Source/WebCore/platform/graphics/TiledBackingStore.cpp:340
> +            // A null trajectory vector (no motion) means that tiles for the keepArea will be created.
> +            coverRect = keepRect;

The comment should say "tiles for the *coverArea*" I think.
It's also a bit confusing to use keepRect first and then assign coverRect to it while all comments refer to the cover area at this point.
I'm not sure how to improve it, maybe by first inflating corevRect at line 334, then assign keepRect = coverRect, and then modify coverRect if necessary in the else at line 341.

[Simon Hausmann]

Comment on attachment 126310 [details]
Patch

ok then, r=me modulo stylistic fixes suggested by jocelyn, me, and the style bot! :)

[Kenneth Rohde Christiansen]

Created attachment 127150 [details]
Patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
Index mismatch: 0dfd183742a71cb5de5dadc3ae177fc72b63a194 != 9cdcda984def14b8bf8a32b6da6784c8a6ef7b3a
rereading 8567f8d3c2539a28a496edaf1048483e973975c2
	M	LayoutTests/fast/forms/radio-nested-labels.html
	M	LayoutTests/ChangeLog
107798 = 3671b2d23de7ade4cb1d1e78a3f6f7673db6a6c9 already exists! Why are we refetching it?
 at /usr/lib/git-core/git-svn line 5210

Died at Tools/Scripts/update-webkit line 164.


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

[WebKit Review Bot]

Comment on attachment 127150 [details]
Patch

Rejecting attachment 127150 [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/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

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

[Simon Hausmann]

Committed r107803: <http://trac.webkit.org/changeset/107803>