Modify

Opened 8 years ago

Closed 8 years ago

#7182 closed enhancement (fixed)

[Patch needs discussion] Individual Icons for Imagery Background

Reported by: simon04 Owned by: team
Priority: normal Milestone:
Component: Core Version:
Keywords: icon Cc:

Description

To make imagery backgrounds easier to find in the (long) Imagery menu list, I suggest to display each an individual icon instead of the default WMS/TMS-icon. This is especially useful if imagery icons are placed in the toolbar.

The attached patch

  1. displays the image specified by logo-image in Maps.
  2. restricts the size of the attribution image being painted to the editing area to 80x80 pixels (this allows to use select images from the "official" servers that are too huge otherwise).
  3. additionally allows to include a menu/toolbar icon in Maps by icon-base64 in a Base64 encoding (this allows preprocessing of the icon). If one does not like a direct inclusion of the icon, we could call use an attribute icon-url.

I assume, either 1+2 or 3 are enough, but both combined give more freedom.

What do you think?

Attachments (6)

7182.patch (8.1 KB) - added by simon04 8 years ago.
7182_v2.patch (11.7 KB) - added by simon04 8 years ago.
7182_v3.patch (10.3 KB) - added by simon04 8 years ago.
7182_v4.patch (10.8 KB) - added by simon04 8 years ago.
7182b.patch (1.8 KB) - added by simon04 8 years ago.
7182-merge.patch (20.0 KB) - added by bastiK 8 years ago.

Download all attachments as: .zip

Change History (29)

Changed 8 years ago by simon04

Attachment: 7182.patch added

comment:1 Changed 8 years ago by bastiK

Base64 would be nice. In HTML you can write

<img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUA
AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
9TXL0Y4OHwAAAABJRU5ErkJggg==">

as well as

<img src="http://www.example.com/images/dot.png">

This is something we can support as well - ideally in a more general way: For preset/mappaint icons and so on.

Can you use javax.xml.bind.DatatypeConverter.parseBase64Binary, rather than proprietary API?

You can replace either width or height by "-1" to preserve the aspect ratio of the image when resizing.

Please don't call it icon-url - this is inconsistent with current element names.

comment:2 Changed 8 years ago by stoecker

I don't like the base64 variant a lot, as the size of the file is already growing due to the area descriptions, but on the other hand it reduces delays caused by loading URL's. So maybe we should try it and see what results are. Anyway bastiK's idea sounds like a better implementation we probably could use elsewhere also (TaggingPresets f.e.).

comment:3 in reply to:  1 Changed 8 years ago by simon04

Replying to bastiK:

Base64 would be nice. In HTML you can write

<img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUA
AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
9TXL0Y4OHwAAAABJRU5ErkJggg==">

as well as

<img src="http://www.example.com/images/dot.png">

This is something we can support as well - ideally in a more general way: For preset/mappaint icons and so on.

I assume that there isn't an implementation in JOSM at the moment? Here is an implementation: http://www.java2s.com/Open-Source/Java-Document/Testing/htmlunit-2.7/com/gargoylesoftware/htmlunit/protocol/data/DataUrlDecoder.java.htm

Can you use javax.xml.bind.DatatypeConverter.parseBase64Binary, rather than proprietary API?

Of course.

Please don't call it icon-url - this is inconsistent with current element names.

What about menu-icon/toolbar-icon or just icon?


We would then allow to denote an image in one of the following two ways:

<icon>data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUA
AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
9TXL0Y4OHwAAAABJRU5ErkJggg==</icon>
<!-- or -->
<icon>http://www.example.com/images/dot.png</icon>

Changed 8 years ago by simon04

Attachment: 7182_v2.patch added

comment:4 Changed 8 years ago by simon04

Some intermediate result:

comment:5 Changed 8 years ago by stoecker

There are some println debugs left. Do we really need to use a third base64, this time from sun? We already have a own implementation and the Apache one.

Changed 8 years ago by simon04

Attachment: 7182_v3.patch added

comment:6 in reply to:  2 Changed 8 years ago by simon04

Removed debugging output, switched to Apache's Base64 implementation. I think this patch can be committed now.

Replying to stoecker:

I don't like the base64 variant a lot, as the size of the file is already growing due to the area descriptions, but on the other hand it reduces delays caused by loading URL's. So maybe we should try it and see what results are.

The support for Base64-encoded images is then available for all features that use the ImageProvider. I'll test how this affects the presets concerning size and start time.

comment:7 in reply to:  5 Changed 8 years ago by bastiK

Replying to stoecker:

Do we really need to use a third base64, this time from sun? We already have a own implementation and the Apache one.

The code is Java 6 now, so we should stick to standard API javax.xml.bind.DatatypeConverter unless there is a problem.

comment:8 Changed 8 years ago by stoecker

We use the line

i = new ImageIcon(i.getImage().getScaledInstance(MAX_ICON_SIZE, MAX_ICON_SIZE, Image.SCALE_SMOOTH)); 

in serveral places, but it is wrong. It does not keep the aspect ratio. We should add a new function to ImageProvider:

   static ImageIcon getScaledImage(ImageIcon icon, int width, int height) {
       if(icon.getIconWidth() > icon.getIconHeight()) {
           return new ImageIcon(icon.getImage().getScaleinstance(width, -1, Image.SCALE_SMOOTH));
       } else {
           return new ImageIcon(icon.getImage().getScaleinstance(-1, height, Image.SCALE_SMOOTH));
       }
   }

and use this instead.

The code is Java 6 now, so we should stick to standard API javax.xml.bind.DatatypeConverter unless there is a problem.

This would be ok, but "sun...." was not good. We can't get rid of the Apache stuff so easily, so the current version is also fine I think.

comment:9 Changed 8 years ago by bastiK

As far as rfc2397 is concerned, we could even support inline svg like this:

data:image/svg+xml,%3Csvg%3E%3Crect%20x=%2210%22%20y=%2210%22%20height=%22100%22%20width=%22100%22%20style=%22fill:%20red%22%2F%3E%3C%2Fsvg%3E

or

data:image/svg+xml;base64,...

comment:10 in reply to:  8 Changed 8 years ago by bastiK

Replying to stoecker:
We should add a new function to ImageProvider:

   static ImageIcon getScaledImage(ImageIcon icon, int width, int height) {
       if(icon.getIconWidth() > icon.getIconHeight()) {
           return new ImageIcon(icon.getImage().getScaleinstance(width, -1, Image.SCALE_SMOOTH));
       } else {
           return new ImageIcon(icon.getImage().getScaleinstance(-1, height, Image.SCALE_SMOOTH));
       }
   }

I'll do it.

The code is Java 6 now, so we should stick to standard API javax.xml.bind.DatatypeConverter unless there is a problem.

This would be ok, but "sun...." was not good. We can't get rid of the Apache stuff so easily, so the current version is also fine I think.

Every class we don't include from Apache reduces size of the binary.

comment:11 Changed 8 years ago by bastiK

In [4712/josm]:

limit the maximum size of the image, but keep the aspect ratio (see #7182)

Changed 8 years ago by simon04

Attachment: 7182_v4.patch added

comment:12 Changed 8 years ago by simon04

Patch version 4:

  • Added support for SVG
  • Use changes from r4712 for scaling

Any further comments?


To ease the usage of ImageProvider, I'd like to add a builder. The usage then would be something like

ImageIcon icon = new ImageProvider().withName(name).withDirs(dirs).withMaxSize(maxSize).getIfAvailable();

We could then deprecate the getIfAvailable methods in favour of this new builder.

comment:13 in reply to:  12 ; Changed 8 years ago by bastiK

Replying to simon04:

Patch version 4:

  • Added support for SVG
  • Use changes from r4712 for scaling

Any further comments?

Looks good.

To ease the usage of ImageProvider, I'd like to add a builder. The usage then would be something like

ImageIcon icon = new ImageProvider().withName(name).withDirs(dirs).withMaxSize(maxSize).getIfAvailable();

We could then deprecate the getIfAvailable methods in favour of this new builder.

Something like this?

comment:14 in reply to:  13 Changed 8 years ago by simon04

Replying to bastiK:

To ease the usage of ImageProvider, I'd like to add a builder. The usage then would be something like

ImageIcon icon = new ImageProvider().withName(name).withDirs(dirs).withMaxSize(maxSize).getIfAvailable();

We could then deprecate the getIfAvailable methods in favour of this new builder.

Something like this?

Exactly this. No need to re-implement this. :-) I think we should favour the usage of this builder and deprecate the complicated (wrt. number of arguments) getIfAvailable methods. What do you think?

Without the support of an IDE it is very hard to figure out which code part belongs to which argument in MapPaintStyles:

ImageIcon i = ImageProvider.getIfAvailable(getIconSourceDirs(ref.source), "mappaint."+namespace, null, ref.iconName, ref.source.zipIcons, width == -1 && height == -1 ? null : new Dimension(width, height), sanitize);

comment:15 Changed 8 years ago by simon04

In [4713/josm]:

see #7182 - Individual Icons for Imagery Background

comment:16 Changed 8 years ago by simon04

For the conversion to Base64-encoded data, one may find this link helpful: http://webcodertools.com/imagetobase64converter/

comment:17 Changed 8 years ago by bastiK

In [4714/josm]:

change wording for ImageRequest builder (see #7182)

comment:18 Changed 8 years ago by bastiK

The vast majority of ImageProvider use is like this

ImageProvider.get("ok");
ImageProvider.get("dialogs", "delete");

I think there is no need to make this more complicated. What we could do, is merge ImageRequest with ImageProvider, write

public static ImageIcon ImageProvider.get(String name) {
   return new ImageProvider().setName(name).get();
}

and get rid of the method with the extremely long argument list (use local fields instead).

comment:19 in reply to:  18 Changed 8 years ago by simon04

Replying to bastiK:

I think there is no need to make this more complicated.

I agree.

What we could do, is merge ImageRequest with ImageProvider, write

public static ImageIcon ImageProvider.get(String name) {
   return new ImageProvider().setName(name).get();
}

If I get it correctly, the implementation ImageProvider.get would use ImageRequest? However, then ImageProvider depends on ImageRequest and vice versa.

and get rid of the method with the extremely long argument list (use local fields instead).

What do you man by "use local fields instead"?


Should ImageRequest.get print a debug message to stdout in the case that the image does not exist (cf. patch)?

As name is required for the use of ImageRequest, I think name should already be set in the constructor, i.e., have ImageRequest(String name) and remove the default constructor.

Changed 8 years ago by simon04

Attachment: 7182b.patch added

comment:20 Changed 8 years ago by simon04

In [4715/josm]:

see #7182 - Individual Icons for Imagery Background (make loading more robust)

Changed 8 years ago by bastiK

Attachment: 7182-merge.patch added

comment:21 Changed 8 years ago by bastiK

See patch. Any disadvantages when the builder is not an extra class, but part of the class?

comment:22 Changed 8 years ago by bastiK

In [4721/josm]:
use builder pattern for ImageProvider

comment:23 in reply to:  21 Changed 8 years ago by simon04

Resolution: fixed
Status: newclosed

Replying to bastiK:

See patch. Any disadvantages when the builder is not an extra class, but part of the class?

Nice. :-)

I think this ticket can be closed.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.