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

fix (web): panorama / 360 view - partial panorama: use photo-sphere-viewer #6992

Merged
merged 3 commits into from Feb 9, 2024

Conversation

dmitry-brazhenko
Copy link
Contributor

@dmitry-brazhenko dmitry-brazhenko commented Feb 9, 2024

Previous PR: #3412

This PR should fix 2 issues: #3490 and #3465

I changed 360 images viewer. Now I use photo-sphere-viewer.

I checked "cropped/partial" panoramas and they work fine.

image

@dmitry-brazhenko dmitry-brazhenko changed the title Panorama (360 view) fixes fix (web): panorama / 360 view - partial panorama: use photo-sphere-viewer Feb 9, 2024
@dmitry-brazhenko dmitry-brazhenko marked this pull request as ready for review February 9, 2024 11:33
@dmitry-brazhenko
Copy link
Contributor Author

@danieldietzler could you help me to merge this PR?

@danieldietzler
Copy link
Member

@danieldietzler could you help me to merge this PR?

I don't merge PRs, only some people do :)
Why is it so urgent? It's not like we'll do a release right after this PR

@dmitry-brazhenko
Copy link
Contributor Author

No urgency, just wanted not to forget about this :)

@danieldietzler
Copy link
Member

We usually don't "forget" pull requests, no worries.

@mertalev
Copy link
Contributor

mertalev commented Feb 9, 2024

The photo-sphere-viewer documentation says that it handles cropped/partial panoramas by looking at image metadata. We strip most metadata from preview images now, so can you confirm that a newly uploaded partial panorama still works?

@alextran1502 alextran1502 merged commit 2ee9044 into immich-app:main Feb 9, 2024
23 checks passed
@alextran1502
Copy link
Contributor

@mertalev tested on panorama taken on iPhone and it works

@dmitry-brazhenko
Copy link
Contributor Author

Yep, I also tested on newly uploaded images. As far as I see, exif is not removed from image

@rovo89
Copy link
Contributor

rovo89 commented Feb 9, 2024

@dmitry-brazhenko I think panorama-viewer.css is now unused, isn't it? Then I guess it should be deleted.

@dmitry-brazhenko
Copy link
Contributor Author

@rovo89 thanks a lot for your attentiveness!!

Here is a PR: #7012

@waclaw66
Copy link
Contributor

A few comments...

  • panorama photos got attributes InitialViewHeadingDegrees and InitialViewPitchDegrees, ignoring them causes that image is rotated to the wrong side initially, they should be kept and set to poseHeading and posePitch panorama viewer properties
  • there is a white vertical line between info panel and panorama, it's because viewport of panorama has partial dimensions, but the viewer uses integer (truncated) ones

obrazek

@dmitry-brazhenko
Copy link
Contributor Author

That's good point. I created an issue: #7243

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants