Pre-scaling images to speed up feedback in Image Editor.

Pre-scaling images to speed up feedback in Image Editor.

Also refactored image_buffer.js to split in several loosely coupled objects.

BUG=
TEST=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=95824

[SeRya, 2011-08-08 13:26:43]

LGTM with minor comments.

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
File chrome/browser/resources/file_manager/js/image_editor/image_util.js
(right):

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
chrome/browser/resources/file_manager/js/image_editor/image_util.js:210: dstRect
= dstRect || new Rect(context.canvas);
Shouldn't it be dstRect_opt ans srcRect_opt?

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
File chrome/browser/resources/file_manager/js/image_editor/viewport.js (right):

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
chrome/browser/resources/file_manager/js/image_editor/viewport.js:26: 
New line should not be here. The argument should be declared in jsdoc.

Shouldn't scaleControl_ be set in constructor to null? It would allow to declare
it's type and would preserve the hidden class.

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
chrome/browser/resources/file_manager/js/image_editor/viewport.js:44:
Viewport.prototype.getScale = function () { return this.scale_ };
Space before ().

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
chrome/browser/resources/file_manager/js/image_editor/viewport.js:46:
Viewport.prototype.setScale = function (scale, notify) {
Space before ()

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
chrome/browser/resources/file_manager/js/image_editor/viewport.js:117: * Access
to the current viewport state.
It's not a JS Doc comment and dont need to comply with its style.

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
chrome/browser/resources/file_manager/js/image_editor/viewport.js:159: /*
/** ?

[Vladislav Kaznacheev, 2011-08-08 13:47:50]

Thanks for the review, committing.

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
File chrome/browser/resources/file_manager/js/image_editor/image_util.js
(right):

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
chrome/browser/resources/file_manager/js/image_editor/image_util.js:210: dstRect
= dstRect || new Rect(context.canvas);
On 2011/08/08 13:26:44, SeRya wrote:
> Shouldn't it be dstRect_opt ans srcRect_opt?

Done.

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
File chrome/browser/resources/file_manager/js/image_editor/viewport.js (right):

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
chrome/browser/resources/file_manager/js/image_editor/viewport.js:26: 
Initialized in the constructor. 

As for the comment: this was meant as a section comment, not a JSDoc, so I
removed a second *

On 2011/08/08 13:26:44, SeRya wrote:
> New line should not be here. The argument should be declared in jsdoc.
> 
> Shouldn't scaleControl_ be set in constructor to null? It would allow to
declare
> it's type and would preserve the hidden class.

Done.

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
chrome/browser/resources/file_manager/js/image_editor/viewport.js:44:
Viewport.prototype.getScale = function () { return this.scale_ };
On 2011/08/08 13:26:44, SeRya wrote:
> Space before ().

Done.

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
chrome/browser/resources/file_manager/js/image_editor/viewport.js:46:
Viewport.prototype.setScale = function (scale, notify) {
On 2011/08/08 13:26:44, SeRya wrote:
> Space before ()

Done.

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
chrome/browser/resources/file_manager/js/image_editor/viewport.js:117: * Access
to the current viewport state.
On 2011/08/08 13:26:44, SeRya wrote:
> It's not a JS Doc comment and dont need to comply with its style.

Done.

http://codereview.chromium.org/7541075/diff/1/chrome/browser/resources/file_m...
chrome/browser/resources/file_manager/js/image_editor/viewport.js:159: /*
Also a section comment.
On 2011/08/08 13:26:44, SeRya wrote:
> /** ?

[commit-bot: I haz the power, 2011-08-08 16:30:58]

Change committed as 95824