#21016 closed defect (fixed)
[Patch] ColorfulFilter not applied to empty tiles
Reported by: | Bjoeni | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | 21.06 |
Component: | Core imagery | Version: | tested |
Keywords: | template_report color filter | Cc: | michael2402, simon04 |
Description (last modified by )
The ColorfulFilter
is not applied to tiles that appear to be "empty" (maybe tiles that do not contain ways or are below a certain size?)
Relative:URL: ^/trunk Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b Last:Changed Date: 2021-06-02 22:03:39 +0200 (Wed, 02 Jun 2021) Revision:17919 Build-Date:2021-06-02 20:11:30 URL:https://josm.openstreetmap.de/svn/trunk Identification: JOSM/1.5 (17919 de) Windows 10 64-Bit OS Build number: Windows 10 Home 2009 (21390) Memory Usage: 437 MB / 4028 MB (148 MB allocated, but free) Java version: 11.0.10+9, AdoptOpenJDK, OpenJDK 64-Bit Server VM Look and Feel: com.formdev.flatlaf.FlatDarkLaf Screen: \Display0 1920×1080 (scaling 1.00×1.00) \Display1 2560×1440 (scaling 1.00×1.00) Maximum Screen Size: 2560×1440 Best cursor sizes: 16×16→32×32, 32×32→32×32 System property file.encoding: Cp1252 System property sun.jnu.encoding: Cp1252 Locale info: de_DE Numbers with default locale: 1234567890 -> 1234567890 VM arguments: [-Dicedtea-web.bin.location=C:\Program Files\OpenWebStart\javaws, --add-modules=java.scripting,java.sql, --add-exports=java.desktop/com.apple.eawt=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.spi=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.plugins.jpeg=ALL-UNNAMED, --add-exports=javafx.graphics/com.sun.javafx.application=ALL-UNNAMED, --add-exports=jdk.deploy/com.sun.deploy.config=ALL-UNNAMED, --add-opens=java.base/java.lang=ALL-UNNAMED, --add-opens=java.base/java.nio=ALL-UNNAMED, --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED, --add-opens=java.base/jdk.internal.ref=ALL-UNNAMED, --add-opens=java.desktop/javax.imageio.spi=ALL-UNNAMED, --add-opens=java.desktop/javax.swing.text.html=ALL-UNNAMED, --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED, -Djava.util.Arrays.useLegacyMergeSort=true, --add-exports=java.base/sun.net.www.protocol.jar=ALL-UNNAMED,java.desktop, --add-exports=java.base/jdk.internal.util.jar=ALL-UNNAMED,java.desktop, --add-exports=java.base/com.sun.net.ssl.internal.ssl=ALL-UNNAMED,java.desktop, --add-reads=java.naming=ALL-UNNAMED,java.desktop, --add-exports=java.desktop/sun.awt.X11=ALL-UNNAMED,java.desktop, --add-exports=java.desktop/sun.applet=ALL-UNNAMED,java.desktop,jdk.jsobject, --add-exports=java.base/sun.security.action=ALL-UNNAMED,java.desktop, --add-reads=java.base=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.net.www.protocol.http=ALL-UNNAMED,java.desktop, --add-exports=java.naming/com.sun.jndi.toolkit.url=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.security.util=ALL-UNNAMED,java.desktop,ALL-UNNAMED, --add-reads=java.desktop=ALL-UNNAMED,java.naming, --add-exports=java.desktop/sun.awt=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.security.x509=ALL-UNNAMED,java.desktop,ALL-UNNAMED, --add-exports=java.desktop/javax.jnlp=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.security.provider=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.security.validator=ALL-UNNAMED,java.desktop] Plugins: + InfoMode (35543) + Mapillary (2.0.0-alpha.12.2) + MicrosoftStreetside (35248) + apache-commons (35524) + apache-http (35589) + flatlaf (35734) + javafx-windows (35655) + jna (35662) + photo_geotagging (35738) + photoadjust (35770) + utilsplugin2 (35691) Last errors/warnings: - 00134.665 W: JCS - Silent failure during download: https://c.tile.opentopomap.org/13/4604/1937.png - 00134.719 W: JCS - Silent failure during download: https://a.tile.opentopomap.org/13/4595/1939.png - 00134.762 W: JCS - Silent failure during download: https://a.tile.opentopomap.org/13/4595/1936.png - 00136.002 W: JCS - Silent failure during download: https://b.tile.opentopomap.org/13/4603/1940.png - 00136.776 W: JCS - Silent failure during download: https://a.tile.opentopomap.org/13/4595/1941.png - 00136.795 W: JCS - Silent failure during download: https://c.tile.opentopomap.org/13/4597/1940.png - 00138.326 W: JCS - Silent failure during download: https://a.tile.opentopomap.org/13/4598/1934.png - 00139.275 W: JCS - Silent failure during download: https://a.tile.opentopomap.org/13/4595/1940.png - 00139.281 W: JCS - Silent failure during download: https://c.tile.opentopomap.org/13/4604/1936.png - 00140.782 W: JCS - Silent failure during download: https://c.tile.opentopomap.org/13/4603/1935.png
Attachments (7)
Change History (27)
by , 3 years ago
Attachment: | colorful1.png added |
---|
by , 3 years ago
Attachment: | colorful2.png added |
---|
comment:1 by , 3 years ago
Description: | modified (diff) |
---|
comment:2 by , 3 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:3 by , 3 years ago
@Bjoeni Feel free to fix it.
getRGB should not be that much of an performance overhead - it is just a memory overhead. But modern JVMs are pretty good at handling such big short-lived arrays. A problem would be calling getRGB for every single pixel.
comment:4 by , 3 years ago
I don't know how getRGB works internally, but I guess somehow each pixel needs to be converted to the resulting color int. That's what I was most afraid of, in terms of performance overhead.
I didn't specifically check the memory overhead, but as you said I too think it shouldn't really be much higher than the current implementation. Maybe I can change the individual a/r/g/b values to bytes, that might save something. Theoretically it shouldn't matter though, because I think any primitive should be freed immediately at the end of each iteration.
I'll have another look sometime over the next few days.
comment:5 by , 3 years ago
This won't save you anything - those primitive variables are assigned to registers. Each of them occupies a 64 bit register, no matter what you declare them. If you use a byte, you would need extra operations to avoid the sign extensions, so using ints (or long) here is perfectly fine. But don't optimize what the compiler will optimize for your ;-)
comment:7 by , 3 years ago
Milestone: | → 21.06 |
---|---|
Owner: | changed from | to
Status: | assigned → new |
Summary: | ColorfulFilter not applied to empty tiles → [Patch] ColorfulFilter not applied to empty tiles |
- add filtering for more image types, including Mapnik tiles and TIFF files
- move filter processing in
ImageDisplay
to separate thread, unblocking GUI - add unit test for
BufferedImage.TYPE_BYTE_BINARY
by , 3 years ago
Attachment: | 21016.patch added |
---|
comment:8 by , 3 years ago
P.S. I'm just setting the milestones to not completely lose track of the patches, as there are currently like 9 or 10 of my patches outstanding.
@team, obviously feel free to push the milestones as necessary.
follow-up: 11 comment:9 by , 3 years ago
Welcome to the club - I don't even have track to some of the patches outstanding from me. There is one nice one (that one that allows imagery to be used on every projection by re-projecting it), which is pending for ~ 5 years…
comment:10 by , 3 years ago
Well to be fair those outstanding patches are all from within the last 1.5 months, as I've been a bit more active around here lately. And there are also some tickets that I can't resolve because it would conflict with another patch.
which is pending for ~ 5 years…
I'm just trying to avoid exactly that ;)
follow-ups: 12 13 comment:11 by , 3 years ago
Replying to michael2402:
Welcome to the club - I don't even have track to some of the patches outstanding from me. There is one nice one (that one that allows imagery to be used on every projection by re-projecting it), which is pending for ~ 5 years…
??? We have imagery reprojection in josm for years?
Well to be fair those outstanding patches are all from within the last 1.5 months, as I've been a bit more active around here lately. And there are also some tickets that I can't resolve because it would conflict with another patch.
Vincent has slowed down and I have other issues ATM. For this special case ATM: If you have ready to be used patches overlooked, please add them to current milestone.
comment:12 by , 3 years ago
Vincent has slowed down and I have other issues ATM.
I know and I'm not trying to "blame" anyone for anything, I really appreciate the work you guys put into it! Simon was nice enough to take a look / commit some of my patches lately, but without him not much has been happening the last couple of weeks. Again, not blaming anyone (we're all doing it just for fun), I'm just trying to keep track of the patches in the meantime so they don't get lost.
For this special case ATM: If you have ready to be used patches overlooked, please add them to current milestone.
That's exactly what I did: I set the milestone on all of my ready patches right away, to avoid them being overlooked. Because otherwise I'm probably gonna forget them myself.
by , 3 years ago
Attachment: | Bildschirmfoto von 2021-06-23 20-31-17.png added |
---|
Imagery Reprojection (2)
by , 3 years ago
Attachment: | Bildschirmfoto von 2021-06-23 20-31-08.png added |
---|
Imagery Reprojection compared to OSM data
by , 3 years ago
Attachment: | Bildschirmfoto von 2021-06-23 20-27-16.png added |
---|
Some bugs in imagery reprojection.
follow-up: 16 comment:13 by , 3 years ago
Replying to stoecker:
Replying to michael2402:
Welcome to the club - I don't even have track to some of the patches outstanding from me. There is one nice one (that one that allows imagery to be used on every projection by re-projecting it), which is pending for ~ 5 years…
??? We have imagery reprojection in josm for years?
Yes. Still some bugs (especially when zoomed out or in some corner cases and some Performance problems when you have a big difference in tile sizes, see last image)
(Sorry for spamming this thread, I would have to search the ticket this originally belonged to)
follow-up: 18 comment:16 by , 3 years ago
Replying to michael2402:
(Sorry for spamming this thread, I would have to search the ticket this originally belonged to)
Reprojection has been implemented in #7427.
I already noticed some glitches in #16082. Help would be very welcome there. I completely failed to understand the reprojection code back then and was unable to fix the issues, although I'd love to see them gone.
comment:18 by , 3 years ago
Replying to Don-vip:
I already noticed some glitches in #16082. Help would be very welcome there. I completely failed to understand the reprojection code back then and was unable to fix the issues, although I'd love to see them gone.
This is a very different approach from what I have been doing.
I used a mesh (not triangles but squares, thus the strange artifacts) an reprojected while drawing the original tiles. The current algorithm creates new tiles that are generated from the original imagery. It creates sort of a fake imagery source, which requires a more complex state management.
comment:19 by , 3 years ago
@skyper, could you please confirm if #21062 is fixed as well for the images that didn't work before?
The issue appears to be that some tiles are provided as
BufferedImage.TYPE_BYTE_BINARY
.Related tickets: #14763 and #15878
At the time it was decided that it wasn't necessary to support further image types, as the conversion would make JOSM unresponsive. But I just ran some tests and with the following method that I wrote I actually got quite good results - despite using
getRGB()
:I can't really give you specific numbers, as it depends on the underlying image (and we're talking about timeframes of a few ms per tile), but for images already present as
BufferedImage.TYPE_INT_ARGB
I got pretty much the same results as for the existing method, and for images present asBufferedImage.TYPE_BYTE_BINARY
it took ~3 times as long. That however still means less than 12ms/tile on my laptop, which is even with quick zooming/panning absolutely not noticeable for the user. Even when I forced JOSM to run both methods on each tile to compare how long they took, I didn't notice any difference.This method also works with
BufferedImage.TYPE_CUSTOM
, which became more relevant since the filters can be applied to the image viewer since #20659. There however I could notice some delay with filters applied to big images (like TIFFs > 100MB, guess that's not JOSM's typical use case anyways). But if anything I think that means we should implement the subsampling mentioned in ticket:20813#comment:9 and/or threading.I'll play around with that a bit more, and see if I can optimize it further.
So @michael2402, if you don't have any objections I'll provide a patch to use this method as a fallback to support any kind of tiles.