Reviewers: jgw,

Message:
Joel,
  This review isn't super high-priority and is kind of long.  I got a
more than just a bit irritated at how hard it was to try to make what
should have been a simple change to ImageResourceGenerator and decided
that the code needed a rework.  If you think that someone else might
have more cycles to look at it, please let me know who can take a look
at it.

http://code.google.com/p/google-web-toolkit/issues/detail?id=4418


http://gwt-code-reviews.appspot.com/335802/diff/1/3
File
user/src/com/google/gwt/resources/rebind/context/AbstractClientBundleGenerator.java
(right):

http://gwt-code-reviews.appspot.com/335802/diff/1/3#newcode85
user/src/com/google/gwt/resources/rebind/context/AbstractClientBundleGenerator.java:85:
factory.addName(ident);
NameFactory doesn't add the identifiers to the pool that it creates.
I'm not sure if that's intentional or a bug.

http://gwt-code-reviews.appspot.com/335802/diff/1/4
File user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java
(left):

http://gwt-code-reviews.appspot.com/335802/diff/1/4#oldcode221
user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java:221:
interface HasRect {
Removed this interface because it is silly.

http://gwt-code-reviews.appspot.com/335802/diff/1/4
File user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java
(right):

http://gwt-code-reviews.appspot.com/335802/diff/1/4#newcode377
user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java:377: public
AffineTransform transform() {
Updated transform() with scaling.

http://gwt-code-reviews.appspot.com/335802/diff/1/4#newcode623
user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java:623: rect =
addImage(logger, imageName, resource);
No longer doing caching by content hash in ImageBundleBuilder; this has
been moved into ImageResourceGenerator.LocalizedImage.

http://gwt-code-reviews.appspot.com/335802/diff/1/5
File user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java
(left):

http://gwt-code-reviews.appspot.com/335802/diff/1/5#oldcode55
user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java:55:
static class CachedState {
The Maps in CachedState have mostly been moved into proper data
structures.

http://gwt-code-reviews.appspot.com/335802/diff/1/5#oldcode92
user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java:92:
static class LocalState {
LocalState has been removed.  The little bit of state here was moved
into DisplayedImage.normalContentsFieldName and rtlContentsFieldName.

http://gwt-code-reviews.appspot.com/335802/diff/1/5#oldcode158
user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java:158:
ClientBundleFields fields) throws UnableToCompleteException {
This code is moved into the implementations of DisplayedImage.render().

http://gwt-code-reviews.appspot.com/335802/diff/1/5#oldcode265
user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java:265:
private void finalizeArrangements(TreeLogger logger)
Moved to BundleImage.render()

http://gwt-code-reviews.appspot.com/335802/diff/1/5#oldcode337
user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java:337:
private boolean getFlipRtl(JMethod method) {
getFlipRtl() and getRepeatStyle() have been moved to
ImageResourceDeclaration.

http://gwt-code-reviews.appspot.com/335802/diff/1/5#oldcode361
user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java:361:
private String maybeDeploy(ResourceContext context,
This code has been moved into the implementations of
DisplayedImage.render().

http://gwt-code-reviews.appspot.com/335802/diff/1/5
File user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java
(right):

http://gwt-code-reviews.appspot.com/335802/diff/1/5#newcode554
user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java:554: *
they actually offer a space-savings.
This fixes a problem where we would de-optimize a PNG that has been run
through a modern compressor.

Description:
This patch reworks ImageResourceGenerator to make its caching much saner
by getting rid of many of the Maps and using a more object-oriented
style of programming.  This should resolve issue 4418 (ImageResources
not being localized).  A sanity check for de-optimizing a PNG file has
been added.  Additionally, the option to provide compile-time scaling
for ImageResources was added as a means of seeing whether or not the new
code structure makes it easier to add features (it does).

Please review this at http://gwt-code-reviews.appspot.com/335802/show

Affected files:
  M user/src/com/google/gwt/resources/client/ImageResource.java
M user/src/com/google/gwt/resources/rebind/context/AbstractClientBundleGenerator.java
  M user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java
  M user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java
  M user/test/com/google/gwt/resources/client/ImageResourceTest.java


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to