[webkit-reviews] review denied: [Bug 66472] [chromium] Add backend compositor support for rescaling (zooming) textures during zoom animation. : [Attachment 105804] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Sep 2 13:04:43 PDT 2011

James Robinson <jamesr at chromium.org> has denied W. James MacLean
<wjmaclean at chromium.org>'s request for review:
Bug 66472: [chromium] Add backend compositor support for rescaling (zooming)
textures during zoom animation.

Attachment 105804: Patch

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=105804&action=review

Thanks for the test.  The code looks pretty sane (although you've got a lot of
merge conflicts now. pardon our dust!)

I'm not really sure what is going on with the test, though.  What would the
test display if the test failed?  Normally we try to construct tests to that
you see red if it fails and green if it passes, if that's feasible, and
otherwise we try to describe the output you should see in the test (in an HTML
comment or the like).  I can't tell if that test is passing or failing just
from looking at it.  It would also be good to have tests for >1 zooms as well
as <1.	I'm also a little confused about whether the non-initialized compositor
blue is showing up in the test or if that's part of the test page.  We probably
shouldn't be showing the uninitialized blue ever, right?

> LayoutTests/platform/chromium/compositing/zoom-animator-scale-test.html:9
> +	  if (window.layoutTestController)
> +	    window.layoutTestController.waitUntilDone();

please also call layoutTestController.dumpAsText(true) to indicate that the
pixel output is the only thing needed

More information about the webkit-reviews mailing list