Proposed API Change: AbstractImage and ClippedImage

[Bruce Johnson]

Motivation
It is very efficient to combine individual images into one large "image strip" then reveal sub-images, via offsets and clipping, as individual image objects. For more background, see the ImageBundle design doc: http://code.google.com/p/google-web-toolkit/wiki/ImageBundleDesign. The design doc (which needs to be updated to reflect this proposal) refers to a "clipping constructor for Image". Instead, this proposal is to create two new classes instead: AbstractImage and ClippedImage.

Example
Use a single image URL to contain all the images used on a toolbar. Each image is a 16x16.

  ClippedImage newFileIcon = new ClippedImage(" toolbar.gif", 0, 0, 16, 16);
  ClippedImage openFileIcon = new ClippedImage("toolbar.gif", 16, 0, 16, 16);
  ClippedImage saveFileIcon = new ClippedImage("toolbar.gif", 32, 0, 16, 16);

The Change
1) Add AbstractImage as a new base class for Image.
2) Update Image to include only differences from AbstractImage.
3) Add ClippedImage that extends AbstractImage.

AbstractImage exposes functionality that is supported by all possible kinds of images. Its API is correspondingly sparse:

  public class AbstractImage extends Widget
      implements SourceClickEvents, SourceMouseEvents {

    // Move the generic prefetch logic here from Image,
    // but still callable as Image.prefetch(...);
    public static void prefetch(String url);

    // In rare cases, subclasses need something other than <img>.
    protected AbstractImage(Element elem);

    public void add/removeClickListener(...);
    public void add/removeMouseListener(...);
  }

Image would extend AbstractImage and also implement SourcesLoadEvents (not all image subclasses can support load events, as some use background images or other tricks):

  public class Image extends AbstractImage implements SourceLoadEvents {
    public Image();
    public Image(String url);
    public void add/removeLoadListener(...);
    public String getUrl();
    public String setUrl(String url);
  }

Note that setUrl()/getUrl() are not necessarily appropriate for all image subclasses and so does not appear on AbstractImage.

ClippedImage is a subclass of AbstractImage. It requires browser-specific tricks, so its API is conservatively sparse:

  public class ClippedImage extends AbstractImage {
    public ClippedImage(String url, int left, int top, int width, int height);
  }

Consequences/Caveats

• Careful use of ClippedImages can drastically improve startup time by reducing the number of HTTP image requests, particularly in combination with ImageBundleGenerator.
  
• ClippedImage probably eventually needs more methods, but they can be added in a subsequent release, driven by use cases.
  
• ClippedImage uses several tricks that prevent "carefree styling" with CSS. In other words, several CSS attributes do not operate properly on ClippedImages, including padding and border. In every case I've thought of, the same effect can be achieved by introducing additional wrapper elements.
  

[John Tamplin]

LGTM.

--
John A. Tamplin
Software Engineer, Google

[Scott Blum]

The one thing is that AbstractImage isn't very... imagey.  I guess I mean, all it really is (if you discount the static methods) is a Widget of some type that sources click and mouse events.  I'm not sure why the load listeners wouldn't be part of AbstractImage.  Or getUrl() for that matter.  What about this?

  public class AbstractImage extends Widget

      implements SourceClickEvents, SourceMouseEvents, SourceLoadEvents {

    // Move the generic prefetch logic here from Image,
    // but still callable as Image.prefetch(...);
    public static void prefetch(String url);

    protected AbstractImage();

    public void add/removeClickListener(...);
    public void add/removeMouseListener(...);

    public void add/removeLoadListener(...);
    public String getUrl();
  }

  public class Image extends AbstractImage implements SourceLoadEvents {
    public Image();
    public Image(String url);

    public String setUrl(String url);

  }

  public class ClippedImage extends AbstractImage {

    public ClippedImage();
    // convenience ctor, like Image has

    public ClippedImage(String url, int left, int top, int width, int height);

    // analagous to Image.setUrl()
    public setUrlAndBounds(String url, int left, int top, int width, int height);
    // for changing size/loc without even trying to fetch a new url
    public setBounds(int left, int top, int width, int height);
    // methods for querying the current left/top/width/height of the image (distinct from the bounds of the DOM element)
    // methods for querying the size of the real (non-clipped) image if this is possible?
  }

A use case for setBounds + getters would be a filmstrip where you update the image location on a Timer.

A use case for setUrlAndBounds would be to change the image without having to rip the old one out of the DOM and create a new img element.

Scott

[Dan Morrill]

I was about to LGTM but then I read Scott's post.  Now I agree with Scott.

- Dan

[Bruce Johnson]

You'd be surprised how very different ClippedImage is from Image. And ClippedImage on IE is totally different than everywhere else. I understand your example of a filmstrip, and there is indeed a private method called setUrlAndBounds() already.

So, we agree in spirit rather than on timing. More methods will very likely be needed in ClippedImage and AbstractImage. But it truly isn't clear which ones should be where at the moment, and I don't think it will become clear until we have a couple more AbstractImage subclasses (or realize we don't need any more).

I'd hate to make a mistake and necessitate an UnsupportedOperationException in some subclass because we generalized too soon.

Why not let's be conservative and generalize the type and add methods only after we have a little experience with it?

-- Bruce

[Scott Blum]

That's a good argument, and one I might be willing to buy.  There is though one good reason I argued for putting more into AbstractImage which I didn't mention.  Our core UI library is meant to essentially reflect the DOM and provide a better API into it.  In a way, the reason we have an Image class is because there is an <img> HTML element.  Same with many of our other core widgets.  I think the reason it makes sense to put more stuff into AbstractImage is that if we ever added in something else that was kind of Image like but didn't contain an <img> element, maybe it shouldn't be an Image at all.  So at the very least, I'd think AbstractImage should have the load listeners.

That being said, I could easily see how the thing you actually send into setElement being something other than an <img>.. it could be an <img> wrapped in an outer div or something.

As far as setUrlAndBounds(), what about instead of making them private, making them protected final?  So someone could at least subclass if they wanted to play around with it.

On 2/1/07, Bruce Johnson <br...@google.com> wrote:

[John Tamplin]

On 2/2/07, Scott Blum <sco...@google.com> wrote:

That's a good argument, and one I might be willing to buy.  There is though one good reason I argued for putting more into AbstractImage which I didn't mention.  Our core UI library is meant to essentially reflect the DOM and provide a better API into it.  In a way, the reason we have an Image class is because there is an <img> HTML element.  Same with many of our other core widgets.  I think the reason it makes sense to put more stuff into AbstractImage is that if we ever added in something else that was kind of Image like but didn't contain an <img> element, maybe it shouldn't be an Image at all.  So at the very least, I'd think AbstractImage should have the load listeners.

Well, the ImagleBundle, at least as we last discussed it over lunch, would actually not be an <img> tag to avoid the padding issue.  However, I think you clearly want the individual images in that to be usable where an image is elsewhere.

[Scott Blum]

On 2/2/07, John Tamplin <j...@google.com> wrote:

Well, the ImagleBundle, at least as we last discussed it over lunch, would actually not be an <img> tag to avoid the padding issue.  However, I think you clearly want the individual images in that to be usable where an image is elsewhere.

Okay, but it would still have to contain some kind of img for which you could get an load event, right?

Scott

[Bruce Johnson]

On 2/2/07, Scott Blum <sco...@google.com> wrote:

You seem to thnk the DOM is somehow not insane? In the case of ClippedImage, the image that you actually care about is associated with a CSS background-image (on non-IE), which isn't the actual src attribute of the <img> tag, so the onload even that you'd get would definitely not be the one you actually care about. On IE, it's buried in a proprietary 'filter' CSS style that has an analogous problem.

That's a perfect example of the problematic generalization that makes me want to be conservative.

[Scott Blum]

I concede!

[Emily Crutcher]

ImageBundles are very similar in flair to Constants.  With Constants, we chose to remove the "get" modifier. Should we do the same thing here? So for the WordProcessorImageBundle, we would have the following interface:

 

interface WordProcessorImageBundle extends ImageBundle {
  /**
   * @gwt.resource newfile.gif
   */
  Image newFileIcon();

  /**
   * @gwt.resource openfile.gif
   */
  Image openFileIcon();

  /**
   * @gwt.resource savefile.gif
   */
  Image saveFileIcon();
}

[Miroslav Pokorny]

Just wondering why are you guys worry about Image strips which are essentially an optimisation but dont have proper support for many lower level abstracts eg - inline styles, computed styles, regular native objects which should be mandatory. Everything you do works with inline/computed styles...

--
mP

[Scott Blum]

On 2/6/07, Miroslav Pokorny <miroslav...@gmail.com> wrote:

Just wondering why are you guys worry about Image strips which are essentially an optimisation but dont have proper support for many lower level abstracts eg - inline styles, computed styles, regular native objects which should be mandatory. Everything you do works with inline/computed styles...

Well, it's a really, really, really nice optimization.  Besides which, our design philosophy to this point has largely been to encourage developers to work in CSS directly to do what they need, rather than doing a bunch of style work in code.

Scott

[mP]

Of course the ImageStrip thing is a nice thing to have. There are a
zillion nice things to have :)

For widget developers there should be a layer around styles etc. All
widgets being written need to address the differences between all
browsers. The DOM class takes care of the nasties for Elements - the
same should exist for styles.

I would vote on getting this done before worrying about ImageStrips.
If one starts from the bottom up, thjese low level abstracts are the
next step.

First the compiler and related runtime support, abstractions over
native objects ( eg DOM support for Event, Element, some support for
styles) and in turn ytou use the bits below to build higher level
abstractions (widgets!).

On Feb 7, 7:48 am, "Scott Blum" <sco...@google.com> wrote:

[Scott Blum]

This line of debate really has nothing to do with the subject of this thread.  You should start a new thread if you want to have a general discussion of development priorities, or of explicit style support in particular.