Skip to content

[visionOS] Relax criteria for Panoramic images to delegate fullscreen to Quick Look#35329

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
yoelhawa:eng/136329595
Oct 18, 2024
Merged

[visionOS] Relax criteria for Panoramic images to delegate fullscreen to Quick Look#35329
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
yoelhawa:eng/136329595

Conversation

@yoelhawa
Copy link
Copy Markdown

@yoelhawa yoelhawa commented Oct 16, 2024

42e1ee2

[visionOS] Relax criteria for Panoramic images to delegate fullscreen to Quick Look
https://bugs.webkit.org/show_bug.cgi?id=281632
<rdar://136329595>

Reviewed by Said Abou-Hallawa.

On visionOS, delegate element fullscreen to Quick Look for any image
that is wide enough to be a Panoramic image, even if it isn't large
enough. That way, if `srcset` switches the image source to one large
enough to be a Panoramic image after going fullscreen, Quick Look
will already be open to display it.

* Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:
(WebCore::ImageDecoderCG::isMaybePanoramic const):
(WebCore::ImageDecoderCG::shouldUseQuickLookForFullscreen const):
(WebCore::ImageDecoderCG::isPanoramic const): Deleted.
* Source/WebCore/platform/graphics/cg/ImageDecoderCG.h:
Loosen the criteria for panorama detection.

Canonical link: https://commits.webkit.org/285388@main

f12527b

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@yoelhawa yoelhawa self-assigned this Oct 16, 2024
@yoelhawa yoelhawa added the Images For bugs in image handling. label Oct 16, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 17, 2024
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the size of the image is small like { 30 x 10 } and there is no srcset around this image in the page, will opening QuickLook in this case be okay?

Should not we make the decision to use QuickLook at a higher level where more context about the image is known?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can be more refined with the heuristic, but there's little risk to using Quick Look for fullscreen (it now presents an identical visual experience to the user as regular element fullscreen)

@etiennesegonzac etiennesegonzac added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Oct 18, 2024
… to Quick Look

https://bugs.webkit.org/show_bug.cgi?id=281632
<rdar://136329595>

Reviewed by Said Abou-Hallawa.

On visionOS, delegate element fullscreen to Quick Look for any image
that is wide enough to be a Panoramic image, even if it isn't large
enough. That way, if `srcset` switches the image source to one large
enough to be a Panoramic image after going fullscreen, Quick Look
will already be open to display it.

* Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp:
(WebCore::ImageDecoderCG::isMaybePanoramic const):
(WebCore::ImageDecoderCG::shouldUseQuickLookForFullscreen const):
(WebCore::ImageDecoderCG::isPanoramic const): Deleted.
* Source/WebCore/platform/graphics/cg/ImageDecoderCG.h:
Loosen the criteria for panorama detection.

Canonical link: https://commits.webkit.org/285388@main
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 285388@main (42e1ee2): https://commits.webkit.org/285388@main

Reviewed commits have been landed. Closing PR #35329 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 42e1ee2 into WebKit:main Oct 18, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Oct 18, 2024
@yoelhawa yoelhawa deleted the eng/136329595 branch October 18, 2024 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Images For bugs in image handling.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants