Modify

#16201 closed enhancement (fixed)

[Patch] Simplify calculation of SVG size in ImageProvider

Reported by: floscher Owned by: team
Priority: minor Milestone: 18.05
Component: Core Version:
Keywords: Cc:

Description

This is a small patch improving how the ImageProvider calculates the x and y scaling factors for an SVG:

  • the scaled width and height as well as the scaling factors are now final variables that are assigned only once.
  • all negative numbers in the desired image dimensions are treated the same (that dimension is scaled automatically). Before only -1 was treated this way, other negative values were treated as the desired width/height.
  • the logo sizes are now square (128×129 → 128×128 and 256×258 → 256×256)

Attachments (4)

ImageProvider-SVG-scaling.patch (2.7 KB) - added by floscher 13 months ago.
ImageProvider-SVG-scaling-2.patch (2.3 KB) - added by floscher 13 months ago.
Follow-up patch for the previous patch: Fixes calculation of scaling factor which caused functional test to fail.
test-output.png (46.3 KB) - added by floscher 13 months ago.
Output of the way-repeat-image test after the second patch
test-differences.png (398 bytes) - added by floscher 13 months ago.
Difference between the reference image and the output of way-repeat-image test after the second patch

Download all attachments as: .zip

Change History (13)

Changed 13 months ago by floscher

comment:1 Changed 13 months ago by floscher

ping

comment:2 Changed 13 months ago by Don-vip

Milestone: 18.05

comment:3 in reply to:  description Changed 13 months ago by Don-vip

Replying to floscher:

  • the logo sizes are now square (128×129 → 128×128 and 256×258 → 256×256)

I don't remember why we (I?) did that but it was intentional and wasn't a typo. Did you change them to fix a specific issue?

comment:4 Changed 13 months ago by floscher

I think (can't check right now), the logo was not exactly square until I modified the SVG logo and amongst other things made the size of the image exactly square by adjusting the blank space around the logo.
I'll check that later.

comment:5 Changed 13 months ago by floscher

Ok, I see. You added the 128×129 dimension in https://josm.openstreetmap.de/changeset/7768/josm . At that time the PNG version of the logo was 256×258, but the SVG version was 1000×1000px in size at all times.

So it should be ok to make the dimensions square, now that only the SVG version is used.

comment:6 Changed 13 months ago by Don-vip

Resolution: fixed
Status: newclosed

In 13723/josm:

fix #16201 - Simplify calculation of SVG size in ImageProvider (patch by floscher)

comment:7 Changed 13 months ago by Don-vip

Resolution: fixed
Status: closedreopened

This breaks org.openstreetmap.josm.gui.mappaint.MapCSSRendererTest.testRender[way-repeat-image]:

jenkins/job/JOSM/4141/jdk=JDK8/testReport/junit/org.openstreetmap.josm.gui.mappaint/MapCSSRendererTest/testRender_way_repeat_image_/

Changed 13 months ago by floscher

Follow-up patch for the previous patch: Fixes calculation of scaling factor which caused functional test to fail.

comment:8 Changed 13 months ago by floscher

Thank you for the notification. The issue was, that I calculated the scaling factor based on the width/height of the resulting raster image (integer) and the size of the unscaled SVG (floating point number). It's now calculated from the size of the scaled SVG as floating point number and the size of the unscaled SVG also as floating point number.

The test org.openstreetmap.josm.gui.mappaint.MapCSSRendererTest.testRender[way-repeat-image] still fails, but the differences are really minimal (30 pixels differ by 18 or less). These are probably rounding artifacts, because changed the calculation a bit around so rounding happens at different places than before.

New output:
Output of the `way-repeat-image` test after the second patch
Diff to reference image:
Difference between the reference image and the output of `way-repeat-image` test after the second patch

Last edited 13 months ago by floscher (previous) (diff)

Changed 13 months ago by floscher

Attachment: test-output.png added

Output of the way-repeat-image test after the second patch

Changed 13 months ago by floscher

Attachment: test-differences.png added

Difference between the reference image and the output of way-repeat-image test after the second patch

comment:9 Changed 13 months ago by Don-vip

Resolution: fixed
Status: reopenedclosed

In 13737/josm:

fix #16201 - Fixes calculation of scaling factor (patch by floscher)

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.