Set KML icon size to 32x32 by default

[lowswaplab]

Description of the Change

Set a default height/width if missing in image. For example, Font Awesome SVGs do not have height or width set and this causes and error.

Why Should This Be In Core?

So icon sets like Font Awesome and other SVGs can be used as icons.

Benefits

Font Awesome and other SVGs can be used as icons.

Potential Drawbacks

Unknown.

Applicable Issues

None.

[ComBatVision]

Hi, @lowswaplab

Could you attach some screenshots which explaining the nature of the changes. For example how this problem with Font Awesome SVGs looks like. Why developer can not or should not specify dimensions manually in this case? And why default size is 32x32?

[lowswaplab]

[lowswaplab]

I picked 32x32 as a reasonable icon size.

The first image is in Firefox without the "if width/height is 0, set to 32". WorldWind throws an error saying that image size is less than 1 and no icon is drawn. The second image is in Firefox setting the missing width/height and the SVG icon is drawn properly.

I just tried it in Chrome and Chrome sets an SVG missing width/height to 256x256.

I'm not sure there any any good solutions to this... The browsers behave differently. We could simply make the default 256 instead of 32. But hardcoding values is not ideal or reliable long term.

Have any good ideas how to deal with SVGs that don't have width/height explicitly set?

[ComBatVision]

How do you use this SVG icons? Is it possibly to specify image size manually when you load them?

I am asking because I am out of scope of this functionality.

[lowswaplab]

In KML there is not a way to specify the icon size other than scale.

`

Le tour Eiffel

    <visibility>1</visibility>
    <Style>
      <IconStyle>
        <scale>1</scale>
        <Icon>
          <href>http://localhost/smile.svg</href>
        </Icon>
        <color>FF00FF00</color>
      </IconStyle>
    </Style>
    <Point>
      <extrude>1</extrude>
      <altitudeMode>absolute</altitudeMode>
      <coordinates>2.294,48.8858222,200</coordinates>
    </Point>
  </Placemark>

`

[ComBatVision]

Is this issue related to some geo content file parsing? So, may be it is better to setup default size not to general texture itself, but in the code where parser creates placemark?

[lowswaplab]

I had the same thought. The texture is created by image.onload() in GpuResourceCache.prototype.retrieveTexture(). I need to trace it back further to the KML functions...

[ComBatVision]

It is too global change to set default texture size to 32x32, but it is acceptable to set default size for importable placemarks from KML.

I propose to go this way.

[lowswaplab]

For KML, Google Earth normalizes all icons to 32x32. Then the tag increases the size accordingly.

The KML tag has and to specify size of icon. I'm going to look at using and to specify the height/width of the icon.

[lowswaplab]

I found a much better way to do this. Parse KML icon width and height tags and set the icon size accordingly. What do you think?

[lowswaplab]

Here's a summary of what I did:

• Added imageH and imageW to PlacemarkAttributes
• Added optional height/width options to GpuResourceCache because that's where the image is actually loaded and the only place we can set the image's height/width
• Parse KML and get icon width/height, set imageH and imageW
• Placemark now has imageH and imageW and calls GpuResourceCache to load the image

[ComBatVision]

It is not a trivial modification. I need some time to think abut your approach.

By the way, is it not an issue of SVG file itself that it has no pixel size specified?

[lowswaplab]

Height and width fields are not required in SVG

[ComBatVision]

Ok. I will try to review your PR this weekend. I spend end of previous week to merge upstream into Java repository, thats why did not look at your code yet.

[lowswaplab]

No problem! Thanks!

[ComBatVision]

I have thought a lot about this issue...

And I do not like an idea to extend GpuResourceCache.retrieveTexture function and PlacemarkAttributes with additional width and height attributes, which are required only for solving current SVG issue. It is too global change in the architecture for so small issue. But I can not propose any other solution to do it better then you did.

I have several questions to discuss: 1) Can we make workaround for this issue not from the WorldWnd side, but from SVG side? I mean can we force developer to add width and height in SVG as a requirement? Why it is not a good solution from your point of view?

2) How do you think will it be useful to set custom width and height for raster images using your code if we accept it? I mean do you know any use case when we will use e.g. 1024x1024 JPEG or PNG image, but will set its custom size to 32x32 or 2048x2048 instead of controlling scale? What this possibility may give to developer? I am asking this to find any global explanation for adding width and height to any PalcemarkAttributes and to understand why WorldWind architects did not add this possibility at the very beggining.

3. Will system resize or cut raster image from uper left corner if we will apply width and height change onLoad as you did for SVG?

Theoretically width and height are not conflicting with scale and gives developer more flexibility, but I really can not understand why it was not implemented in initial design.

I will try to discuss this concept with NASA official team.

[lowswaplab]

I completely understand your concerns.

1. The SVG standard does not require height/width. We probably cannot convince every developer to implement it in their SVGs.

2. This gives the developer the ability to work with SVGs. There are a huge amount of SVG icons available on the Internet. This patch also further implements the KML standard, making WorldWind support for KML more complete. If support for and are also added, then icon palettes can be support (see gx:y).

3. SVGs are resized to the appropriate height/width.

Also, KML standardizes all icons to 32x32 and then uses scale to size them accordingly. To be KML compliant, then WorldWind should maybe also scale KML icons to 32x32.

Finally, adding width/height options to GpuResourceCache doesn't break any other code. The width/height are optional and ignored if not supplied as a function parameter.

[ComBatVision]

Hi. In my 2 and 3 questions I have asked not only about SVG but also about raster images.

How your improvement will work with jpeg and png? Will it resize or crop images to specified width and height?

Could you answer my comments in your commit code, please. I found one part of code which looks like useless.

[lowswaplab]

I tested the code with .jpg, .gif, .png and all images were resized according to the gx:w and gx:h given in a KML file.

What code is useless? I may have accidentally committed it... I will remove it...

[ComBatVision]

About useless code. Is this part required? if (style) { if (style.kmlIconStyle) { if (style.kmlIconStyle.kmlIcon) { if (style.kmlIconStyle.kmlH && style.kmlIconStyle.kmlH > 0) placemarkAttributes.imageH = style.kmlIconStyle.kmlH; if (style.kmlIconStyle.kmlW && style.kmlIconStyle.kmlW > 0) placemarkAttributes.imageW = style.kmlIconStyle.kmlW; }; }; };

It looks like KMLStyle code in 156-157 lines already fill width and height in placemark attributes.

[lowswaplab]

Are "style", "style.kmlIconStyle", and "style.kmlIconStyle.kmlIcon" guaranteed to always exist? If so, then the code is useless. If not, this checks to see if they exist before referencing them

It looks like some error checking is done upstream from prepareAttributes() for "style". So the check for "style" is not needed. I looked upstream further and I believe the checks for "style.kmlIconStyle" and "style.kmlIconStyle.kmlIcon" are still required.

[ComBatVision]

No, I meaned does this whole part of code is required? As I can se from the code, you already copy kml style into PlacemarkAttributesin another class.

Why you copy it second time? Look at my comments inside your previous commit.

[lowswaplab]

I now see what you're talking about. I removed the code in KmlPlacmark.js and setting image height/width still works.

[ComBatVision]

Yes, this is what i meant. I do not know why they redefine offset and other settings there.

[lowswaplab]

I set the default height/width to 32x32 to be consistent with Google Earth. This means that a user's KML does not have to explicitly always set gx:h and gx:w.

[lowswaplab]

In my next pull request, I will be implementing support for the KML IconStyle heading tag. This allows rotating the icon by degrees relative to north.

[ComBatVision]

I just read KML specification and I think we have a trouble: The , , , and elements are used to select one icon from an image that contains multiple icons (often referred to as an icon palette). https://developers.google.com/kml/documentation/kmlreference#gxw

According to this we can not use and to specify source icon size. It is designed to cut icon from a pallet, not to resize whole source image.

Lets may be have some meeting in any messenger and discuss how Google Earth handle such case?

[lowswaplab]

I agree with you on proportional resizing and the icon pallet comments. I've been thinking about what to do...

[ComBatVision]

We need to find another approach.

Could you tell me if you know: 1) How GoogleEarth works with SVG image files without size? Does it display them 32x32? 2) How GoogleEarth displays raster images of size different from 32x32? Does it resize all images to 32x32 or displays them in original size?

[lowswaplab]

Wow... Google Earth (on Linux at least) does not support SVG icons! JPG, PNG, BMP are listed in the dialog box where you can select a custom icon. I was able to load GIFs as icons too.

I took a random 310 wide 175 tall PNG and used it as an icon. The icon, on Google Earth, was resized to be 32 pixels tall with proportional width. It was not made to be 32x32.

The scale tag resizes the icon, not gx:w or gx:h.

[ComBatVision]

Hm... Interesting information.

If Google Earth does not support SVG, why we need to support it in the KML scope of WWD? What is the usecase?

Have you tried to save KML in Google Earth, replace some PNG with your SVG in text of XML and load it back? What it says or how it looks?

To make the similar behavior in WWD the only way is to: 1) Calculate proportional size in GPUResourceCache.loadTexture instead of just assigning it to image. Use this size as is if image has no size. 2) Pass constant 32 width and height using PlacemarkAttributes in KML style.

But in this case scale up will lost image quality.

Theoretically image should be loaded as is, but viewed as 32x32 with scale 1 and if we set scale 2 it should not be upscale from 32x32, but downscale from 310x175.

[lowswaplab]

I don't think we need to support empty SVGs in WorldWind. I don't think KML supports SVG XML text inside. KML expects a URL.

So now we should ask: Should we set icons to 32px height by default like Google Earth does? Or just delete this whole fork?

I will try a couple of different things. I'll try to force height to 32px for every KML icon and then try different scale amounts and see how it looks.

[ComBatVision]

I meant what if you replace URL to PNG file with URL to SVG file and try to open it in Google Earth? Will it be an error or empty placemark or what?

If it will be error or empty placemark, then you are right and supporting SVG in WWD KML processing will be overengineering.

Your experiment with 32x32 is interesting. Will wait for result.

[lowswaplab]

Oh, I see. I tried an SVG URL and Google Earth displays an X with a box around it.

I'll try the 32 things soon...

[lowswaplab]

I set just height to 32 (width set to null). The image didn't scale proportionately. It was elongated because width didn't scale.

So in GpuResourceCache for image.onload, I scale the image's width according to change in height.

[lowswaplab]

I just tried setting KML scale to 5 to bring the image back up close to the 310x175 original size and it looks great!

The image started to pixelate when I set the KML scale to 10.

[ComBatVision]

So, do you propose to pass 32x32 from KMLStyle via PlacemarkAttributes to GPUResourceCache and resize image onLoad proportionally to fit maximal dimension to be 32?

How to name attributes in PlacemarkAttributes? MaxWidth and MaxHeigh or InitialWith and InitialHeight or OnLoadWidth and OnLoadHeight?

[lowswaplab]

I pass height=32 and width=null in KMLStyle.

Right now, it's called _imageH and _imageW in PlacemarkAttributes. I can name it anything you want. How about displayHeight and displayWidth? renderHeight & renderWidth? We want to show that it's not the actual image's height/width

[ComBatVision]

Passing width only will work incorrectly for images which has height bigger than width. Image height will be more than 32.

As for naming, its meaning is - initial size on image load before scale. Display or render height is not informative, because these terms better fits resulted image size after scale (resulted display or render size).

We need to give more meaningfully names than imageWidth and ImageHeight, to give developer understanding that it will be initially resized to this size proportionnally on image load and not simply resized to its size.

[lowswaplab]

If height is null and width is 32, now I scale height. I tested it and it works.

[lowswaplab]

initialHeight and initialWidth are okay with me.

[ComBatVision]

1) Let's name PlacemarkAttributes and loadImage function parameters as imageInitialWidth and imageInitialHeight to comply with imageScale naming and put them together before imageScale in code. 2) Current code in GPUResourceCache is incorrect. It should accept both parameters (width and heigth), not only one of them, then calculate the scale rate of width/initialWidth and height/initialHeight, pick the biggest rate and use it for both sides of image. In this case image will always fit specified limits. If only one parameter is specified than use its rate for both sides. 3) Lets do new PR with new title "Set initial size of KML Placemark to 32x32" with clean code of this changes without many commits which rollback changes of each other.

[lowswaplab]

Ok. let me keep checking in here until we agree upon what should go in the new PR... I think it'll be easier for me to keep track of everything.

[ComBatVision]

Ok. No problem. Did you understand p.2?

If you wish I can commit this change myself without PR. I already understood how it should be.

[lowswaplab]

I think I understand #2.

[lowswaplab]

Ok, this appears to work. Please check the logic.

[ComBatVision]

The logic in GPUResourceCache is generally correct, but to complicated.

Could you optimize it like this (or I can do it myself during code review when merge): 1) If initialWidth parameter is set, than calculate ratioW else make it equals 0. 2) If initialHeight parameter is set than calculate ratioH else make it equals 0. 3) Pick bigger ratio (including the case when one of ratio is 0 because initial parameter was not specified). 4) Use bigger ratio (if it is not 0) only once to calculate result image size (this will calculate all three cases).

Do you agree?

[ComBatVision]

Furthermore, what about to cover the initial case with SVG for usual (non-KML) placemarks - if loaded image has no width and height, than use initial dimensions as loaded image size, otherwise calculate proportional size described above?

var hasInitialWidth = typeof(initialWidth) === "number" && initialWidth > 0; 
var hasInitialHeight = typeof(initialHeight) === "number" && initialHeight > 0;

if (image.width || image.height) {
    // Resize image proportionally to fit initial size
    var ratioW = hasInitialWidth ? image.width / initialWidth : 0; 
    var ratioH = hasInitialHeight ? image.height / initialHeight : 0;
    var ratio = ratioH > ratioW ?  ratioH : ratioW;
    if (ratio > 0) {
        image.width = image.width / ratio;
        image.height = image.height / ratio;
    }
} else if (hasInitialWidth && hasInitialHeight) {
    // Set initial size of image (SVG image may have no size specified)
    image.width = initialWidth;
    image.height = initialHeight;
}

[ComBatVision]

@lowswaplab could you, please, test the branch https://github.com/WorldWindEarth/worldwindjs/tree/enhancement/initial-image-size

I have implemented discussed logic there.

Thanks.

[lowswaplab]

I tested that branch. The icons appear at their normal size. They do not start out at 32 height (with width scaled). I briefly looked at the diff but couldn't see why it's not working.