Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancement: view mode for 360-degree panorama photo #79

Merged
merged 12 commits into from Jul 14, 2018
Merged

Enhancement: view mode for 360-degree panorama photo #79

merged 12 commits into from Jul 14, 2018

Conversation

miurahr
Copy link
Contributor

@miurahr miurahr commented Jun 21, 2018

  • Implement 360 degree panorama photo viewer mode
  • Shows a photo image as following data flow;
    image -(projection mapping)-> offscreenImage buffer -> screen
    instead of normal mode;
    image -> screen
  • Closes issue 3d viewing support for spherical/360°/omni photos  #56
  • Limitations
    • Disables a zoom feature
    • Use if(is360) conditionals. It is better to
      separate two mode as individual classes in future.
    • Fixes visibleRect, which control zoom,as same as offscreenImage size.
      that is also keep as same as screen size.
    • Disables a drag feature
    • No indication the view direction on the edit pain.
    • No OpenGL accelaration, it may be very slow or consume many CPU
      cycles.
  • Recognize as 360 degree photo which thum image is same or larger than 2048px
    width and 2:1 ratio.
    • It do not see EXIF/MXP property because thums don't have it.
    • There are no button to toggle 360 degree photo mode and a normal mode.

Signed-off-by: Hiroshi Miura miurahr@linux.com

- Implement 360 degree panorama photo viewer mode
- Shows a photo image as following data flow;
  `image` -(projection mapping)-> `offscreenImage` buffer -> screen
  instead of normal mode;
  `image` -> screen
- Closes issue #56
- Limitations
  - Disables a zoom feature
  - Use `if(is360)` conditionals. It is better to
    separate two mode as individual classes in future.
  - Fixes `visibleRect`, which control zoom,as same as `offscreenImage` size.
   that is also keep as same as screen size.
  - Disables a drag feature
  - No indication the view direction on the edit pain.
  - No OpenGL accelaration, it may be very slow or consume many CPU
    cycles.
- Recognize as 360 degree photo which thum image is same or larger than 2048px
  width and 2:1 ratio.
  - It do not see EXIF/MXP property because thums don't have it.
  - There are no button to toggle 360 degree photo mode and a normal mode.

Signed-off-by: Hiroshi Miura <miurahr@linux.com>
@codecov-io
Copy link

codecov-io commented Jun 21, 2018

Codecov Report

Merging #79 into master will increase coverage by 1.73%.
The diff coverage is 43.38%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #79      +/-   ##
============================================
+ Coverage     39.94%   41.67%   +1.73%     
- Complexity      536      582      +46     
============================================
  Files            89       92       +3     
  Lines          4126     4302     +176     
  Branches        549      573      +24     
============================================
+ Hits           1648     1793     +145     
- Misses         2346     2367      +21     
- Partials        132      142      +10
Flag Coverage Δ Complexity Δ
#model_and_api 100% <100%> (ø) 150 <0> (ø) ⬇️
Impacted Files Coverage Δ Complexity Δ
...reetmap/josm/plugins/mapillary/MapillaryLayer.java 33.2% <0%> (ø) 27 <0> (ø) ⬇️
...josm/plugins/mapillary/MapillaryImportedImage.java 41.17% <100%> (ø) 11 <1> (ø) ⬇️
.../josm/plugins/mapillary/gui/panorama/Vector3D.java 100% <100%> (ø) 5 <5> (?)
...josm/plugins/mapillary/gui/panorama/UVMapping.java 100% <100%> (ø) 2 <2> (?)
...gins/mapillary/utils/api/JsonSequencesDecoder.java 100% <100%> (ø) 30 <0> (ø) ⬇️
...reetmap/josm/plugins/mapillary/MapillaryImage.java 64.51% <100%> (+3.22%) 13 <0> (+1) ⬆️
...osm/plugins/mapillary/gui/MapillaryMainDialog.java 62.66% <28.57%> (+18.97%) 26 <0> (+12) ⬆️
...m/plugins/mapillary/gui/MapillaryImageDisplay.java 26.91% <31.15%> (-2.07%) 11 <1> (+2)
...sm/plugins/mapillary/gui/panorama/CameraPlane.java 50.74% <50.74%> (ø) 9 <9> (?)
...josm/plugins/mapillary/MapillaryAbstractImage.java 87.5% <66.66%> (-1.21%) 27 <1> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fefdbc...e96a558. Read the comment docs.

@floscher floscher added this to Image display in General development Jun 21, 2018
@floscher
Copy link
Member

@miurahr Thank you very much for this pull request, I'll definitely look into this.

About the detection of 360° images: We can get this info from the API, there's a pano value that indicates this. Currently that value is discarded and not used by the Mapillary plugin. We can add this later, for now it's good enough to distinguish by size and aspect ratio.

@miurahr miurahr changed the title Enhancement: view mode for 360-degree panorama (WIP; technical preview) Enhancement: view mode for 360-degree panorama (Early stage preview) Jun 21, 2018
Signed-off-by: Hiroshi Miura <miurahr@linux.com>
Signed-off-by: Hiroshi Miura <miurahr@linux.com>
Signed-off-by: Hiroshi Miura <miurahr@linux.com>
Signed-off-by: Hiroshi Miura <miurahr@linux.com>
@miurahr
Copy link
Contributor Author

miurahr commented Jun 23, 2018

Now we see pano API parameter and propagate it to MapillaryImageDisplay class through MapillaryAbstractImage getPano() method.
MapillaryImageDisplay.this.pano is a boolean member to keep status.

Even it got pano is true, if thum size is smaller than 800x400, force to set false and show it as is.

Also fixes bug that it ignore offscreenImage when update graphics, results show an image strange.

- Move panorama related classes to new package 'panorama'
- Add unit test for CameraPlane class.

Signed-off-by: Hiroshi Miura <miurahr@linux.com>
Signed-off-by: Hiroshi Miura <miurahr@linux.com>
Signed-off-by: Hiroshi Miura <miurahr@linux.com>
- Change view vector with right button dragging.
- Zoom drag with left button.
- Right click change view vector where is center.
- Middle button to reset zoom level to 1:1.
- Calculate new view rotation when mouse is released

Signed-off-by: Hiroshi Miura <miurahr@linux.com>
@miurahr
Copy link
Contributor Author

miurahr commented Jun 26, 2018

I believe it becomes good enough features to merge.

@miurahr miurahr changed the title Enhancement: view mode for 360-degree panorama (Early stage preview) Enhancement: view mode for 360-degree panorama photo Jun 26, 2018
@miurahr
Copy link
Contributor Author

miurahr commented Jul 12, 2018

I've built a plugin for here and placed on https://bintray.com/osmfj/JOSM/Mapillary
Does anyone want to test it and give feedback?

@eneerhut
Copy link

eneerhut commented Jul 12, 2018

@miurahr very exciting.

I have got it working successfully. It's slow but seems to work!

Some suggestions:

  1. It would be good to have some UI feedback at this stage that the panorama mode is loading.
    image
    At this point I wasn't aware it was working.

  2. We should see if we can get 360 images to render differently when the imagery layer is enabled as they do on Mapillary.com.

  3. Another option is a toggle in filter to highlight 360 images.

  4. The high-res version of the image does not seem to load for me.
    image

Great progress on this! Sharing with the team now.

@floscher
Copy link
Member

floscher commented Jul 12, 2018

I'll merge this pull request today, currently working on some additions before doing the merge:

  • drop the size requirement for images, when the pano flag is set to true the image should always display spherically
  • render 360° images differently on the map (similar to the website)
  • support for resizing of the image panel
  • draw image detections (e.g. traffic signs) correctly on top of the image

edit: I'm postponing this until tomorrow.

@floscher floscher merged commit e96a558 into JOSM:master Jul 14, 2018
@floscher
Copy link
Member

@eneerhut 4) is probably the same as #60 .

@eneerhut
Copy link

Good point @floscher. Let me draw attention to this.

@miurahr
Copy link
Contributor Author

miurahr commented Jul 18, 2018

@eneerhut Could you open new issues for these points? I'd like to follow up and propose improvements one by one from where I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
General development
Image display
Development

Successfully merging this pull request may close these issues.

None yet

4 participants