Opened 6 years ago
Last modified 6 weeks ago
#16472 assigned enhancement
Add support to 360 / spherical image
Reported by: | StephaneP | Owned by: | Don-vip |
---|---|---|---|
Priority: | major | Milestone: | |
Component: | Core image mapping | Version: | |
Keywords: | 360, panorama, spherical sotmfr2019 | Cc: | taylor.smock |
Description (last modified by )
There are more and more spherical or partial spherical images available, but Josm doesn't display these pictures correctly.
Some ressources :
https://wiki.panotools.org/Panorama_Viewers
https://webuser.fh-furtwangen.de/~dersch/
https://github.com/mpetroff/pannellum
https://openseadragon.github.io/
https://github.com/JOSM/Mapillary/pull/79
https://github.com/spatialdev/MicrosoftStreetside (trac) (based on mapillary)
Attachments (13)
Change History (91)
by , 6 years ago
Attachment: | spherical image inside josm.JPG added |
---|
comment:1 by , 6 years ago
Component: | Core → Core image mapping |
---|
comment:2 by , 6 years ago
Type: | defect → enhancement |
---|
comment:3 by , 6 years ago
Description: | modified (diff) |
---|
comment:4 by , 6 years ago
Priority: | normal → major |
---|
comment:5 by , 6 years ago
comment:6 by , 6 years ago
The Mapillary plugin now displays 360° images: https://github.com/JOSM/Mapillary/pull/79
comment:7 by , 6 years ago
Description: | modified (diff) |
---|
comment:8 by , 6 years ago
Description: | modified (diff) |
---|
comment:9 by , 5 years ago
Keywords: | sotmfr2019 added |
---|
comment:10 by , 5 years ago
New ressources: pv - a free open source multi platform image and panorama viewer (C++)
https://bitbucket.org/kfj/pv/src/master/
comment:11 by , 4 years ago
Any update regarding this feature? It would help to have a common library or functionality in JOSM that we could use for visualizing 360 images.
comment:12 by , 4 years ago
There are at least two plugins dealing with 360° images: Mapillary and Streetside, Do we really need an other implementation? Wouldn't it make more sense to add missing functionality in one of them?
comment:13 by , 4 years ago
I was thinking more about, having only 1 common implementation in JOSM core and everyone else just using it. Instead of adding the same functionality to a third plugin.
comment:14 by , 4 years ago
It depends on the technical dependencies (libraries). If based on JavaFX, it can't be yet included in JOSM core and must be included in a plugin (could be the JavaFX plugin). Then all plugins can depend on it. Otherwise, If Swing-based, it's ok for JOSM core.
comment:15 by , 3 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
follow-up: 18 comment:17 by , 3 years ago
Cc: | added |
---|
@taylor I've never looked at 360 before. Can you please confirm that:
- Mapillary 360 viewer code is in
org.openstreetmap.josm.plugins.mapillary.gui.panorama.*
- StreetSide 360 viewer code is in
org.openstreetmap.josm.plugins.streetside.gui.imageinfo.ThreeSixtyDegreeViewerPanel
- they have nothing in common
- Mapillary code is more advanced / robust
comment:18 by , 3 years ago
Replying to Don-vip:
@taylor I've never looked at 360 before. Can you please confirm that:
- Mapillary 360 viewer code is in
org.openstreetmap.josm.plugins.mapillary.gui.panorama.*
Partially. Some of it is also in the image viewer dialog. (org.openstreetmap.josm.plugins.mapillary.gui.MapillaryImageDisplay
-- see paintPano
and paintNonPano
).
- StreetSide 360 viewer code is in
org.openstreetmap.josm.plugins.streetside.gui.imageinfo.ThreeSixtyDegreeViewerPanel
That is what it looks like -- I was just looking at it today, since I was intending to try to move the code for one of them into JOSM core.
- they have nothing in common
Pretty much. Mapillary just has one image, Streetside has six. And Mapillary's image is restricted to 2048px in one dimension. Streetside uses JavaFX. Mapillary does not.
- Mapillary code is more advanced / robust
The Mapillary plugin implementation is probably better if you have one image and/or don't want to depend on JavaFX. If you have multiple/higher resolution images (i.e., front/top/left/right/back/down), I think Streetside's implementation is better. But I haven't tested both on Java 11+. The Mapillary implementation is a lot more performant on Java 16 than I remember it being on Java 8.
Right now, I believe the Mapillary plugin calculates where each pixel of the image is, instead of what is in the view. It also looks like the Mapillary website has some kind of tiling, so I was starting to write an image tiler so that (a) we don't need the full image in memory and (b) we can just send the tiles of interest to be painted, and only calculate the 3d location of the pixels in the tile (this would make a large difference).
Issues (IIRC) with:
- Streetside
- Super-fast panning (Mapillary had the same issue, once I made it dynamic -- probably an easy fix)
- Camera can become upside down (Mapillary had the same issue, I think I clamped max y)
- Mapillary
- Performance? (may be fixed by tiling and not running calculations for all pixels)
follow-ups: 21 22 23 comment:19 by , 3 years ago
Ah I thought both required JavaFX. It just depends on org.apache.commons.math3
then? How much work it would be to drop this dependency and recode the required math stuff in org.openstreetmap.josm.tools
?
Right now I'd like to focus on a simple 360 image viewer (single image) with no dependency, so we could include it in core.
We'll look at StreetSide later, I didn't know it was handling 6 images at once.
comment:20 by , 3 years ago
Milestone: | → 21.08 |
---|
comment:21 by , 3 years ago
Replying to Don-vip:
Ah I thought both required JavaFX. It just depends on
org.apache.commons.math3
then? How much work it would be to drop this dependency and recode the required math stuff inorg.openstreetmap.josm.tools
?
Not difficult at all. IIRC, I'm pretty much just using it for fastmath. I was intending to port everything over to use the vector math code in apache commons math, but I never got around to it. Much like I've never gotten around to merging the image viewing code into JOSM core, although it has long been on my wishlist...
Mapillary used to depend on JavaFX, strictly for a user-friendly date picker UI. I switched to a swing-based library to fix a recurring, hard to debug, bug report.
Right now I'd like to focus on a simple 360 image viewer (single image) with no dependency, so we could include it in core.
We'll look at StreetSide later, I didn't know it was handling 6 images at once.
Yeah. StreetSide is pretty much combining each side of a cube into a 360 image. Probably not useful at all for JOSM core (I don't think most contributors have multiple synchronized cameras), unless we generate the cuboid ourselves from a single image.
comment:22 by , 3 years ago
Replying to Don-vip:
We'll look at StreetSide later, I didn't know it was handling 6 images at once.
"Cube map" is one of the standard 360 image projection, but equirectangular is more common for local files.
follow-up: 24 comment:23 by , 3 years ago
Replying to Don-vip:
Ah I thought both required JavaFX. It just depends on
org.apache.commons.math3
then? How much work it would be to drop this dependency and recode the required math stuff inorg.openstreetmap.josm.tools
?
Right now I'd like to focus on a simple 360 image viewer (single image) with no dependency, so we could include it in core.
We'll look at StreetSide later, I didn't know it was handling 6 images at once.
OK. It looks like I'm using the following imports:
org.apache.commons.math3.geometry.euclidean.threed.Rotation
: Used to help show user which direction they are pointed in. Not necessary for the initial implementation, but it is a nice-to-have.org.apache.commons.math3.geometry.euclidean.threed.RotationConvention
: See aboveorg.apache.commons.math3.geometry.euclidean.threed.RotationOrder
: See aboveorg.apache.commons.math3.geometry.euclidean.threed.Vector3D
: There was a custom implementation I binned to use the code in apache commons math. I'm in the process of reverting that change.org.apache.commons.math3.util.FastMath
:%s/FastMath/Math/g
(AKA super easy)
TBH, the main reason why I binned the custom Vector3D is that I was trying to use FastMath
to get more performant code. I believe newer Java versions have more performant code.
comment:24 by , 3 years ago
Replying to taylor.smock:
org.apache.commons.math3.geometry.euclidean.threed.Rotation
: Used to help show user which direction they are pointed in. Not necessary for the initial implementation, but it is a nice-to-have.org.apache.commons.math3.geometry.euclidean.threed.RotationConvention
: See aboveorg.apache.commons.math3.geometry.euclidean.threed.RotationOrder
: See aboveorg.apache.commons.math3.geometry.euclidean.threed.Vector3D
: There was a custom implementation I binned to use the code in apache commons math. I'm in the process of reverting that change.org.apache.commons.math3.util.FastMath
:%s/FastMath/Math/g
(AKA super easy)
Let's be iterative. It's OK for the first step to not have the direction arrow, we can add it in a second time.
comment:25 by , 3 years ago
Yep. Iterative is good.
The code in the Mapillary plugin is poorly documented, and the original code predates my involvement. I've been making changes, but I think some of the classes change interpretations -- I'm having to figure out which convention the original programmer used (physics? math?), since that makes a difference for how the symbol (phi
, theta
) is used. I've been renaming the variables so that they do not depend upon the convention used (physics phi
and math theta
-> azimuthalAngle
, physics theta
and math phi
-> polarAngle
(inclination)), and then documenting "This is the physics phi
/theta
and the math theta
/phi
.
And then there is the question of "how were the coordinate axis defined?" I think they used x
and y
in the image plane.
Anyway, I'll try to start moving it over into the JOSM codebase today. I just don't want it to be poorly documented magic. :)
follow-ups: 27 28 comment:26 by , 3 years ago
Well, I think I'm going to have to implement some kind of tiling -- Mapillary has only returned 2048 × 1024 images (2,097,152 pixels).
My 360 camera gave me an image with 6080 x 3040 pixels (18,483,200 pixels). That is ~9x more, and it does not work well.
In any case, current WIP is here: https://gitlab.com/smocktaylor/josm/-/merge_requests/10.patch
Current things that need to be done:
- Modify IImaveViewer interface to get movement information (this is needed to update rotation information).
- Optimizations
- ???
comment:27 by , 3 years ago
Replying to taylor.smock:
My 360 camera gave me an image with 6080 x 3040 pixels (18,483,200 pixels). That is ~9x more, and it does not work well.
Stéphane gave me an HR sample for testing and it's even bigger: 17020 x 8465 pixels (144,074,300 pixels, 41 Mb). You can find it here: download/tmp/Pano_Vieillevigne_1.jpg
comment:28 by , 3 years ago
Replying to taylor.smock:
In any case, current WIP is here: https://gitlab.com/smocktaylor/josm/-/merge_requests/10.patch
I have some trouble to apply it.
comment:29 by , 3 years ago
I haven't rebased all day. That might be it.
Or it might be how git did the diff (it munges the patches together, and I don't think it simplifies it for patch).
comment:30 by , 3 years ago
Notes on attachment:16472.patch
Known issues:
Panning while zoomed in is kind of jerky (as in, viewport jumps around while dragging (<-, ->, <-, ->)Panning from right to left (<-) creates an empty bar on the right sideFixed. I added a method to ImageDisplay that can be called by IImageViewer implementations (it updates the visible rectangle).When the image display is not in the sidebar (docked), zooming really messes things up. Making the image display docked and zooming once again fixes the issue.Fixed. I added a method to ImageDisplay that can be called by IImageViewer implementations (it updates the visible rectangle).- Full resolution is not displayed (pro: Pano_Viellevigne_1.jpg just works, con: cannot zoom in to full resolution)
- Not related to current patch -- see #20813 for the subsampling change
- Performance at small viewports is great, really starts slowing down at larger viewports. Still usable, just jerky. Java 16 is better than Java 8.
by , 3 years ago
Attachment: | 16472.2.patch added |
---|
WIP: DO NOT APPLY -- this code applies to attachment:16472.patch, and is an attempt to figure out where azimuth/polar angles are being flipped
comment:31 by , 3 years ago
Milestone: | 21.08 → 21.09 |
---|
by , 3 years ago
Attachment: | 16472.3.patch added |
---|
comment:32 by , 3 years ago
Notes on attachment:16472.3.patch
- When resizing window, the orientation of the viewpoint changes -- this is due to something, somewhere, flipping angles. I wouldn't call this a show-stopper (the Mapillary viewport just resets to the the vector set in initialization). This can be removed if necessary (just drop Equirectangular.java#L82 (
temporaryCameraPlane.setRotation(currentRotation);
)).
EDIT:
There should be no plugin incompatibilities -- see https://gitlab.com/smocktaylor/josm/-/pipelines/363568437 (all possible incompatibilities are from the HistoryComboBox changes).
I still need to check and make certain that the tests pass.
EDIT 2: Tests aren't passing -- this is due to a change to make things "work", but which completely ruins the expected values. (this is related to the comment for attachment:16472.2.patch)
by , 3 years ago
Attachment: | 16472.4.patch added |
---|
Working 360 viewer, minimal tests, many tests disabled due to failures, but is usable and can be merged
by , 3 years ago
Attachment: | 16472.5.patch added |
---|
Test work for attachment:16472.4.patch . THIS BREAKS IMAGE PANNING! DO NOT APPLY!
comment:33 by , 3 years ago
I'm going to take a break on this for awhile, and come back with fresh eyes. Most of my time spent this week has been trying to figure out how to fix an issue in CameraPlane due to the tests and expected tests I used for the Vector3D class.
Anyway, if a bus runs over me:
- attachment:16472.4.patch can be applied.
- attachment:16472.5.patch should not be applied. It is largely tests for 16472.4.patch, with fixes for expected test behavior. IMO, the equations are more consistent with what should be reality, but I've got to be missing something. Hopefully it jumps out to me in a week or two. (I've got to fix the camera roll).
follow-up: 35 comment:34 by , 3 years ago
Sorry I didn't have enough time in the past 4 weeks to take a look :(
Should I apply attachment:16472.4.patch, or do you have since a better version?
comment:35 by , 3 years ago
Replying to Don-vip:
Sorry I didn't have enough time in the past 4 weeks to take a look :(
Should I apply attachment:16472.4.patch, or do you have since a better version?
Go ahead and apply attachment:16472.4.patch . I don't currently have a better version. It works, but there is a subtle bug that has been present in the Mapillary code for years. It does not affect JOSM, as it has to do with drawing additional items in the image view (in Mapillary's case, detections).
I still want to fix that bug (it is easy to find, but has been a pain to debug).
follow-up: 37 comment:36 by , 3 years ago
OK. The patch needs an update though, I have some trouble to apply the changes in ImageDisplay.java (no problem for other files).
comment:37 by , 3 years ago
Replying to Don-vip:
OK. The patch needs an update though, I have some trouble to apply the changes in ImageDisplay.java (no problem for other files).
Fixed. :)
follow-ups: 40 41 comment:39 by , 3 years ago
Thanks taylor.smock and Don-vip! That's a good start!
@taylor.smock Which image's metadata is checked to detect if the image is a spherical image?
I found one small regression:
If you use keyboard shortcuts to navigate thru the images and reach the end of the sequence, the viewer displays "no image" and you can't go back to the previous image, you have to click on a picture on the geoimage layer.
follow-up: 42 comment:40 by , 3 years ago
Replying to StephaneP:
Thanks taylor.smock and Don-vip! That's a good start!
@taylor.smock Which image metadata is checked to detect if the image is a spherical image?
I found one small regression:
If you use keyboard shortcuts to navigate thru the images and reach the end of the sequence, the viewer displays "no image" and you can't go back to the previous image, you have to click on a picture on the geoimage layer.
I'm using the XMP GPano:ProjectionType
. See source:trunk/src/org/openstreetmap/josm/data/gpx/GpxImageEntry.java@18246:767-776#L767 .
I didn't see anything in EXIF for panoramic images, and all the test images (not downloaded from Mapillary) that I had used that particular key.
Thanks for the bug report. I thought I checked that, but I'll double-check. While I'm double checking, if you are running a local build, can you check if it exists after running ant clean dist
? Sometimes some bad behavior can creep in when not doing a clean build.
EDIT: You made a change Which image
-> Which image's
. The detection is done on a per-image basis.
comment:41 by , 3 years ago
Replying to StephaneP:
I found one small regression:
If you use keyboard shortcuts to navigate thru the images and reach the end of the sequence, the viewer displays "no image" and you can't go back to the previous image, you have to click on a picture on the geoimage layer.
Reproduced. It looks like the shortcut isn't disabled with the button. I'll look into that today.
follow-up: 44 comment:42 by , 3 years ago
Replying to taylor.smock:
I'm using the XMP
GPano:ProjectionType
. See source:trunk/src/org/openstreetmap/josm/data/gpx/GpxImageEntry.java@18246:767-776#L767 .
I didn't see anything in EXIF for panoramic images, and all the test images (not downloaded from Mapillary) that I had used that particular key.
Perhaps you can OR on this XMP exiftag : ProjectionType=equirectangular
That's what tools like Hugin adds on pano.
Reproduced. It looks like the shortcut isn't disabled with the button. I'll look into that today.
Thanks!
by , 3 years ago
Attachment: | 16472.fix_shortcuts.patch added |
---|
Fix shortcuts (add a listener for enable state change on button creation)
comment:43 by , 3 years ago
In attachment:16472.fix_shortcuts.patch, I add a property change listener to created buttons for the enabled
property. I chose to go the listener route instead of adding btn.getAction().setEnabled
everywhere, since I am much more likely to miss one if I go that route. It should also be more robust in the future so that we don't have to remember to update the enabled state of the action.
It used to not matter since ImageData didn't allow next/previous/first/last to deselect data, but with the change, we were effectively deselecting data at the start/end of the data layer.
follow-up: 45 comment:44 by , 3 years ago
Replying to StephaneP:
Perhaps you can OR on this XMP exiftag :
ProjectionType=equirectangular
That's what tools like Hugin adds on pano.
Do you have a sample picture (i.e., one that doesn't work with current code)? I tried making a pano with Hugin on Mac and it worked just fine.
follow-up: 47 comment:45 by , 3 years ago
Replying to taylor.smock:
Do you have a sample picture (i.e., one that doesn't work with current code)? I tried making a pano with Hugin on Mac and it worked just fine.
I'm sorry, you're right, it seems the blurring tools I've used on these pictures deleted some metadata. I'll try to investigate this morning.
by , 3 years ago
Attachment: | pano_Nanterre_universités_V6MPack_with_distortion.jpg added |
---|
picture not detected as a 360 image
comment:47 by , 3 years ago
Replying to StephaneP:
Replying to taylor.smock:
Do you have a sample picture (i.e., one that doesn't work with current code)? I tried making a pano with Hugin on Mac and it worked just fine.
I'm sorry, you're right, it seems the blurring tools I've used on these pictures deleted some metadata. I'll try to investigate this morning.
I found some pictures which seem embed the right metadata, but is not detected as a 360 image:
comment:48 by , 3 years ago
It is working for me. Are you running a local build? If so, please run svn up; ant clean dist
, and try again. If not, please give me the build information and/or update to today's JOSM latest.
EDIT: And by working for me, I mean with today's josm-latest.jar (curl -LO https://josm.openstreetmap.de/josm-latest.jar ; java -jar josm-latest.jar
).
follow-up: 50 comment:49 by , 3 years ago
I was using r18248
And now....it works. I don't understand.
Ok....got it!
I've added some gps coordinates before uploading the picture here. If I remove these metadata with exiftool -gps:all= your_filename
then it's not detected as a pano anymore.
comment:50 by , 3 years ago
Replying to StephaneP:
I was using r18248
And now....it works. I don't understand.
Ok....got it!
I've added some gps coordinates before uploading the picture here. If I remove these metadata withexiftool -gps:all= your_filename
then it's not detected as a pano anymore.
Well, now I know what the problem is.
JOSM short-circuits reading image metadata when there is no gps tags.
See source:trunk/src/org/openstreetmap/josm/data/gpx/GpxImageEntry.java@18247:733-737#L733 , which comes prior to source:trunk/src/org/openstreetmap/josm/data/gpx/GpxImageEntry.java@18247:767-775#L767 .
It might be worth moving IptcDirectory
and XmpDirectory
items prior to the dirGps
check.
by , 3 years ago
Attachment: | 16472.read_image_type_prior_to_short_circuit.patch added |
---|
Move xmp and iptc parsing to before gps short circuit
follow-up: 53 comment:52 by , 3 years ago
Taylor, coverity detected three new defects, can you please check?
New defect(s) Reported-by: Coverity Scan Showing 3 of 3 defect(s) ** CID 1464108: API usage errors (SWAPPED_ARGUMENTS) ________________________________________________________________________________________________________ *** CID 1464108: API usage errors (SWAPPED_ARGUMENTS) /src/org/openstreetmap/josm/gui/util/imagery/CameraPlane.java: 182 in org.openstreetmap.josm.gui.util.imagery.CameraPlane.setRotation(org.openstreetmap.josm.gui.util.imagery.Vector3D)() 176 /** 177 * Set camera plane rotation by spherical vector. 178 * 179 * @param vec vector pointing new view position. 180 */ 181 public void setRotation(Vector3D vec) { >>> CID 1464108: API usage errors (SWAPPED_ARGUMENTS) >>> The positions of arguments in the call to "setRotation" do not match the ordering of the parameters: * "vec.getPolarAngle()" is passed to "azimuthalAngle" * "vec.getAzimuthalAngle()" is passed to "polarAngle" 182 setRotation(vec.getPolarAngle(), vec.getAzimuthalAngle()); 183 } 184 185 public Vector3D getRotation() { 186 return this.rotation; 187 } ** CID 1464107: SpotBugs: Multithreaded correctness (FB.UG_SYNC_SET_UNSYNC_GET) /src/org/openstreetmap/josm/gui/util/imagery/CameraPlane.java: 185 in org.openstreetmap.josm.gui.util.imagery.CameraPlane.getRotation()() ________________________________________________________________________________________________________ *** CID 1464107: SpotBugs: Multithreaded correctness (FB.UG_SYNC_SET_UNSYNC_GET) /src/org/openstreetmap/josm/gui/util/imagery/CameraPlane.java: 185 in org.openstreetmap.josm.gui.util.imagery.CameraPlane.getRotation()() 179 * @param vec vector pointing new view position. 180 */ 181 public void setRotation(Vector3D vec) { 182 setRotation(vec.getPolarAngle(), vec.getAzimuthalAngle()); 183 } 184 >>> CID 1464107: SpotBugs: Multithreaded correctness (FB.UG_SYNC_SET_UNSYNC_GET) >>> org.openstreetmap.josm.gui.util.imagery.CameraPlane.getRotation() is unsynchronized, org.openstreetmap.josm.gui.util.imagery.CameraPlane.setRotation(double, double) is synchronized. 185 public Vector3D getRotation() { 186 return this.rotation; 187 } 188 189 synchronized void setRotation(double azimuthalAngle, double polarAngle) { 190 // Note: Something, somewhere, is switching the two. ** CID 1464106: SpotBugs: Dodgy code (FB.FE_FLOATING_POINT_EQUALITY) /src/org/openstreetmap/josm/gui/util/imagery/Vector3D.java: 239 in org.openstreetmap.josm.gui.util.imagery.Vector3D.equals(java.lang.Object)() ________________________________________________________________________________________________________ *** CID 1464106: SpotBugs: Dodgy code (FB.FE_FLOATING_POINT_EQUALITY) /src/org/openstreetmap/josm/gui/util/imagery/Vector3D.java: 239 in org.openstreetmap.josm.gui.util.imagery.Vector3D.equals(java.lang.Object)() 233 } 234 235 @Override 236 public boolean equals(Object o) { 237 if (o instanceof Vector3D) { 238 Vector3D other = (Vector3D) o; >>> CID 1464106: SpotBugs: Dodgy code (FB.FE_FLOATING_POINT_EQUALITY) >>> Test for floating point equality. 239 return this.x == other.x && this.y == other.y && this.z == other.z; 240 } 241 return false; 242 } 243 244 @Override
comment:53 by , 3 years ago
Replying to Don-vip:
Taylor, coverity detected three new defects, can you please check?
Will do. I know that at least one of those is "this doesn't make sense, but changing it borks rendering and/or panning".
Specifically, "SWAPPED_ARGUMENTS" in "source:trunk/src/org/openstreetmap/josm/gui/util/imagery/CameraPlane.java#L182 in org.openstreetmap.josm.gui.util.imagery.CameraPlane.setRotation(org.openstreetmap.josm.gui.util.imagery.Vector3D)()".
IIRC, that is one of the things I fixed for tests, but since it ruins rendering, cannot be merged in. I did actually comment in the setRotation(double, double)
method that something, somewhere was switching them, and in the tests patch, I think I do have the arguments reversed.
Which, as I mentioned, ruins rendering, and I haven't figured out how to fix that and make the code make sense (case in point, SWAPPED_ARGUMENTS).
For FB.UG_SYNC_SET_UNSYNC_GET
, I don't know why setRotation
is synchronized. It was introduced in Mapillary e96a558add three years ago by floscher. I don't know about floscher, but I don't think I would remember why I did something like that three months ago, much less three years ago.
We can probably synchronize getRotation
and see what happens. It shouldn't harm performance too much (only call for that method is in Equirectangular
, and is only called by getRotation()
and componentResized(ComponentEvent)
). The getRotation()
method call in Equirectangular
isn't used yet, but probably won't be hammered.
Finally, for FB.FE_FLOATING_POINT_EQUALITY, it is in the equals
method. I'd have to look into what makes sense for those. I'll probably start with 1e-8 for the nearness test. Maybe more, maybe less. But most of the vector values are going to be integers (Vector3D is largely called using image coordinates as inputs). Or I'll just use Utils.equalsEpsilon
.
by , 3 years ago
Attachment: | 16472.coverity_fixes.patch added |
---|
Fix coverity FB.FE_FLOATING_POINT_EQUALITY and FB.UG_SYNC_SET_UNSYNC_GET
follow-up: 56 comment:55 by , 3 years ago
Mmm I just got a crash while panning Pano_Viellevigne_1.jpg:
Revision:18253 Is-Local-Build:true Build-Date:2021-10-06 13:19:51 Identification: JOSM/1.5 (18253 SVN fr) Windows 10 64-Bit OS Build number: Windows 10 Pro 2009 (19042) Memory Usage: 778 MB / 4088 MB (414 MB allocated, but free) Java version: 16.0.1+9, AdoptOpenJDK, OpenJDK 64-Bit Server VM Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel Screen: \Display0 2560×1440 (scaling 1.00×1.00) \Display1 1920×1080 (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: UTF-8 System property sun.jnu.encoding: Cp1252 Locale info: fr_FR Numbers with default locale: 1234567890 -> 1234567890 VM arguments: [-ea, --add-opens=java.base/java.io=ALL-UNNAMED, --add-opens=java.base/java.lang=ALL-UNNAMED, --add-opens=java.base/java.nio=ALL-UNNAMED, --add-opens=java.base/java.text=ALL-UNNAMED, --add-opens=java.base/java.util=ALL-UNNAMED, --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED, --add-opens=java.desktop/java.awt=ALL-UNNAMED, --add-opens=java.desktop/javax.swing.text.html=ALL-UNNAMED, --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED, --module-path=C:\Java\javafx-sdk-16\lib, --add-modules=java.scripting,java.sql,javafx.controls,javafx.media,javafx.swing,javafx.web, -Dfile.encoding=UTF-8] Program arguments: [--debug] Last errors/warnings: - 00000.910 W: extended font config - overriding 'filename.Myanmar_Text=mmrtext.ttf' with 'MMRTEXT.TTF' - 00000.912 W: extended font config - overriding 'filename.Mongolian_Baiti=monbaiti.ttf' with 'MONBAITI.TTF' - 00081.646 E: Handled by bug report queue: java.lang.ArrayIndexOutOfBoundsException: Index 285 out of bounds for length 285 === REPORTED CRASH DATA === BugReportExceptionHandler#handleException: No data collected. Warning issued by: BugReportExceptionHandler#handleException === STACK TRACE === Thread: AWT-EventQueue-0 (22) of main java.lang.ArrayIndexOutOfBoundsException: Index 285 out of bounds for length 285 at org.openstreetmap.josm.gui.util.imagery.CameraPlane.setRotationFromDelta(CameraPlane.java:168) at org.openstreetmap.josm.gui.layer.geoimage.viewers.projections.Equirectangular.mouseDragged(Equirectangular.java:98) at org.openstreetmap.josm.gui.layer.geoimage.ImageDisplay$ImgDisplayMouseListener.mouseDragged(ImageDisplay.java:505) at java.desktop/java.awt.Component.processMouseMotionEvent(Component.java:6665) at java.desktop/javax.swing.JComponent.processMouseMotionEvent(JComponent.java:3360) at java.desktop/java.awt.Component.processEvent(Component.java:6386) at java.desktop/java.awt.Container.processEvent(Container.java:2264) at java.desktop/java.awt.Component.dispatchEventImpl(Component.java:4993) at java.desktop/java.awt.Container.dispatchEventImpl(Container.java:2322) at java.desktop/java.awt.Component.dispatchEvent(Component.java:4825) at java.desktop/java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4934) at java.desktop/java.awt.LightweightDispatcher.processMouseEvent(Container.java:4580) at java.desktop/java.awt.LightweightDispatcher.dispatchEvent(Container.java:4504) at java.desktop/java.awt.Container.dispatchEventImpl(Container.java:2308) at java.desktop/java.awt.Window.dispatchEventImpl(Window.java:2773) at java.desktop/java.awt.Component.dispatchEvent(Component.java:4825) at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:772) at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721) at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:715) at java.base/java.security.AccessController.doPrivileged(AccessController.java:391) at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85) at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:95) at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:745) at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:743) at java.base/java.security.AccessController.doPrivileged(AccessController.java:391) at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85) at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:742) at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203) at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124) at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113) at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109) at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101) at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)
reproduced in debug:
java.lang.ArrayIndexOutOfBoundsException: Index 247 out of bounds for length 247 - from = java.awt.Point[x=275,y=237] - to = java.awt.Point[x=277,y=247] - f1 = [x=-0.12866953370492973, y=0.46361879604792133, z=0.8766423233264704, r=1.0, inclination=-0.1457348292682141, azimuthal=1.088721212255313] - this.vectors[to.x].length = 247
comment:56 by , 3 years ago
Replying to Don-vip:
Mmm I just got a crash while panning Pano_Viellevigne_1.jpg:
And I was unable to reproduce (up/down extremes, full 360). But it should be fairly easy to fix. And there is a check for that sort of thing right before the crash location.
EDIT: About the only way to get that is to have some kind of non-square matrix, which shouldn't happen. But obviously did.
by , 3 years ago
Attachment: | 16472.AIOOBE.patch added |
---|
follow-up: 58 comment:57 by , 3 years ago
it takes some time to reproduce, I scroll / zoom in all directions without problem for about 1 minute before triggering the bug, it's strange.
comment:58 by , 3 years ago
Replying to Don-vip:
it takes some time to reproduce, I scroll / zoom in all directions without problem for about 1 minute before triggering the bug, it's strange.
OK. I'll see if I can repro without the patch and with the patch. I wonder if you have to do the drag at exactly the border (max x/y).
follow-up: 63 comment:62 by , 3 years ago
Replying to Don-vip:
In 18263/josm:
I was going to use compass direction when I got around to implementing that, but I like your method better. :)
follow-ups: 64 65 comment:63 by , 3 years ago
Replying to taylor.smock:
I was going to use compass direction when I got around to implementing that, but I like your method better. :)
Thanks! I started to look at the scaling / performance issue but that's harder. I disabled the subsampling code from #20813 and still couldn't display the image in full resolution, I don't understand yet why.
comment:64 by , 3 years ago
Replying to Don-vip:
Replying to taylor.smock:
I was going to use compass direction when I got around to implementing that, but I like your method better. :)
Thanks! I started to look at the scaling / performance issue but that's harder. I disabled the subsampling code from #20813 and still couldn't display the image in full resolution, I don't understand yet why.
This is due to how zooming works in the image viewer at this time.
What happens right now is we only have one image buffer. The size of the image buffer is determined at time of image load based off of the size of the component. This means that we only have that number of pixels available in that buffered image, so zooming in will only really help when something is not easily visible. This does help work around performance issues by allowing users to make the window smaller for better performance.
This has been on my list of things to fix (I was going to go for a zoom/tiling approach, since AFAIK most street level imagery providers use some kind of tiling for images), but it hasn't been super critical yet.
EDIT: Image loading is really expensive in terms of memory used and time, so we have a single buffered image that we do not modify. At least, this is how it works on the 360 side. I copied/pasted the code for standard images, but I think it works the same way.
by , 3 years ago
Attachment: | 16472.load_all.patch added |
---|
DO NOT APPLY -- load the entire image into memory, with attendent calculations (this is very expensive). There is also a UI bug with this patch.
comment:65 by , 3 years ago
Replying to Don-vip:
Thanks! I started to look at the scaling / performance issue but that's harder. I disabled the subsampling code from #20813 and still couldn't display the image in full resolution, I don't understand yet why.
I've attached a patch that should allow you to load an image at full resolution. If you are using download/tmp/Pano_Vieillevigne_1.jpg, you should (a) be prepared to wait awhile (something like 144,074,300 vector calculations, which is not cheap) and (b) have allocated sufficient memory (around 580 MB).
As noted in the patch, don't apply it. Not only is it not going to work well with large images (due to many calculations), it does mess up zoom.
comment:67 by , 3 years ago
Milestone: | 21.10 → 21.11 |
---|
follow-up: 69 comment:68 by , 3 years ago
Replying to Don-vip:
In 18300/josm:
Is this only a temporary solution? If not, it would be nice to specify the external program similar to the web browser.
comment:69 by , 3 years ago
Replying to skyper:
Replying to Don-vip:
In 18300/josm:
Is this only a temporary solution? If not, it would be nice to specify the external program similar to the web browser.
Actually, I am not sure if this new button needs to be documented.
comment:71 by , 3 years ago
Milestone: | 21.12 → 22.01 |
---|
comment:73 by , 3 years ago
Milestone: | 22.02 → 22.03 |
---|
follow-up: 75 comment:74 by , 3 years ago
I have used the 360 picture viewer embeded in JOSM, it works very well, so first thank you for this first ! :+1:
I have only a little bug to report when I save a JOSM session. When I open pictures in a new JOSM session they are detected as 360 images with a pano view. If I save my session in a JOZ file and open it with JOSM then the picture are not opened as 360 pictures by the viewer but as flat picture.
It would be really useful because extracting geolocation of a big number of picture could take 10 minutes or more.
comment:75 by , 3 years ago
It has been reported in ticket #21884 so i will post my message in the correct ticket.
Replying to Corentin LEMAITRE:
I have used the 360 picture viewer embeded in JOSM, it works very well, so first thank you for this first ! :+1:
I have only a little bug to report when I save a JOSM session. When I open pictures in a new JOSM session they are detected as 360 images with a pano view. If I save my session in a JOZ file and open it with JOSM then the picture are not opened as 360 pictures by the viewer but as flat picture.
It would be really useful because extracting geolocation of a big number of picture could take 10 minutes or more.
comment:76 by , 3 years ago
Milestone: | 22.03 → 22.04 |
---|
comment:77 by , 3 years ago
Milestone: | 22.04 |
---|
comment:78 by , 6 weeks ago
Sorry if I'm misunderstanding something - is this ticket still open to deal with the open items, mentioned in https://josm.openstreetmap.de/ticket/16472#comment:30 (full zoom, performance)?
I assume full image would be loaded only upon zooming in, otherwise resource usage would explode - is that correct?
More ressources :
https://github.com/miurahr/panoramaviewer
https://github.com/leonardo-ono/Java3DSphereImageViewer