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

Progressive JPEG in a composited layer should render while loading #15503

Conversation

Ahmad-S792
Copy link
Contributor

@Ahmad-S792 Ahmad-S792 commented Jul 3, 2023

Progressive JPEG in a composited layer should render while loading
https://bugs.webkit.org/show_bug.cgi?id=249113
rdar://problem/103499397

Reviewed by NOBODY (OOPS!).

Inspired: https://src.chromium.org/viewvc/blink?view=revision&revision=198593

A progressive JPEG image renders while it loads, but currently it does not
when the image is in a composited layer. It should render the same way, layered
or not, so do that.

Although this is required in case of Apple platforms leveraging Core Graphics.
Hence, I added platform flags to disable this behavior for other platforms.
It will lead to improved progressive JPEG loading from in specific scenarios
(i.e., transform: translateZ).

This patch is without test case since it would require serving JPEG
progressively and I am not sure, it would be possible within EWS.

* Source/WebCore/rendering/RenderLayerBacking.cpp:
(RenderLayerBacking::updateImageContents): Add platform flags around 'cachedImage' loading case

b47cafd

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim
βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@Ahmad-S792 Ahmad-S792 added the Layout and Rendering For bugs with layout and rendering of Web pages. label Jul 3, 2023
@Ahmad-S792 Ahmad-S792 self-assigned this Jul 3, 2023
@Ahmad-S792 Ahmad-S792 force-pushed the fix249113-progressive-JPEG-composited branch from 8c7221e to 403f32b Compare July 3, 2023 17:05
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 3, 2023
@Ahmad-S792 Ahmad-S792 force-pushed the fix249113-progressive-JPEG-composited branch from 403f32b to 6f64717 Compare July 3, 2023 17:11
@Ahmad-S792 Ahmad-S792 removed the merging-blocked Applied to prevent a change from being merged label Jul 3, 2023
@@ -3185,9 +3185,11 @@ void RenderLayerBacking::updateImageContents(PaintedContentsInfo& contentsInfo)
if (!image)
return;

#if (!PLATFORM(GTK) || !PLATFORM(WPE) || !PLATFORM(WIN) || !PLATFORM(PLAYSTATION))
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn’t PLATFORM(COCOA) be more appropriate here? Also you’re using || instead of &&

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good shout on && but I want to avoid (COCOCA) because it might be effecting more platforms like MACCATALYST etc., so I opted to do this instead of macOS restriction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@donny-dont - Fixed '&&' bit in next commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Was thinking more on this and is there a reason why ports wouldn’t want this code enabled?

Copy link
Contributor Author

@Ahmad-S792 Ahmad-S792 Jul 3, 2023

Choose a reason for hiding this comment

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

In case of Apple platforms, it requires all images which go through compositor layer to not load immediately but loaded in one go, which defeat purpose of 'progressive' loading for JPEG (if they have filter or transforms applied or something which pushes them to be composited). So for other ports, this will also delay loading, so I am adding this part behind platform flags to not delay loading if they are composited images on other platform and continue to load progressively.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Try loading this https://jsfiddle.net/6y4zjb17/ on Safari vs Chrome.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fujii & @donny-dont - I will wait for @smfr's input to help me understand on which platform, we need to add this flag (e.g., COCOA only or some others as well).

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe an ENABLE flag would be more appropriate here then?

https://bugs.webkit.org/show_bug.cgi?id=249113
rdar://problem/103499397

Reviewed by NOBODY (OOPS!).

Inspired: https://src.chromium.org/viewvc/blink?view=revision&revision=198593

A progressive JPEG image renders while it loads, but currently it does not
when the image is in a composited layer. It should render the same way, layered
or not, so do that.

Although this is required in case of Apple platforms leveraging Core Graphics.
Hence, I added platform flags to disable this behavior for other platforms.
It will lead to improved progressive JPEG loading from in specific scenarios
(i.e., transform: translateZ).

This patch is without test case since it would require serving JPEG
progressively and I am not sure, it would be possible within EWS.

* Source/WebCore/rendering/RenderLayerBacking.cpp:
(RenderLayerBacking::updateImageContents): Add platform flags around 'cachedImage' loading case
@Ahmad-S792 Ahmad-S792 force-pushed the fix249113-progressive-JPEG-composited branch from 6f64717 to b47cafd Compare July 3, 2023 17:30
@Ahmad-S792 Ahmad-S792 closed this Sep 7, 2023
@Ahmad-S792 Ahmad-S792 deleted the fix249113-progressive-JPEG-composited branch September 7, 2023 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layout and Rendering For bugs with layout and rendering of Web pages.
Projects
None yet
4 participants