Bug 111304 - -webkit-image-set should consider Frame::pageZoomFactor()
Summary: -webkit-image-set should consider Frame::pageZoomFactor()
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Allan Sandfeld Jensen
URL:
Keywords:
Depends on:
Blocks: 81698
  Show dependency treegraph
 
Reported: 2013-03-04 05:38 PST by Allan Sandfeld Jensen
Modified: 2014-02-05 11:20 PST (History)
10 users (show)

See Also:


Attachments
Patch (2.72 KB, patch)
2013-03-04 05:41 PST, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (5.29 KB, patch)
2013-03-04 06:11 PST, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (8.00 KB, patch)
2013-03-04 06:50 PST, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (8.59 KB, patch)
2013-03-14 07:37 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Allan Sandfeld Jensen 2013-03-04 05:38:16 PST
-webkit-image-set currently only considers deviceScaleFactor, if it also supported pageZoomFactor it would mean we could pick higher resolution images on zoomed pages.
Comment 1 Allan Sandfeld Jensen 2013-03-04 05:41:15 PST
Created attachment 191211 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2013-03-04 05:59:08 PST
How does this interact with loading. Do we have to always reload both versions, or can we zoom in and end up in the situation that the high res version isnt loaded?
Comment 3 Allan Sandfeld Jensen 2013-03-04 06:11:07 PST
Created attachment 191218 [details]
Patch

Now with a test
Comment 4 Allan Sandfeld Jensen 2013-03-04 06:19:33 PST
(In reply to comment #2)
> How does this interact with loading. Do we have to always reload both versions, or can we zoom in and end up in the situation that the high res version isnt loaded?

We don't load the higher resolution image until it is needed. Essentially this behaves in similar way as if you changed the CSS content property to a new image. So yes, zooming in will cause the image to be loaded. Note that this is WK1 style zooming though, and a full restyle and repaint is also performed on each zoom step.

It does not work with pinch zoom yet, and would require some refactoring before that is possible.
Comment 5 Allan Sandfeld Jensen 2013-03-04 06:50:52 PST
Created attachment 191231 [details]
Patch

Avoid rerequesting images when the scale changes in a way that does not affect the choice of image
Comment 6 Beth Dakin 2013-03-04 11:30:07 PST
I definitely think this feature is interesting, but it should be opt-in somehow. All ports may not want to do this.
Comment 7 Allan Sandfeld Jensen 2013-03-04 12:20:51 PST
(In reply to comment #6)
> I definitely think this feature is interesting, but it should be opt-in somehow. All ports may not want to do this.

That could get complicated with options, this is just the easiest of the fixme's for image-set. The other include responding to pageScaleFactor and even CSS transforms. I was under the impression the intention of the image-set feature was exactly to be able to do this at some point.
Comment 8 Allan Sandfeld Jensen 2013-03-14 07:37:54 PDT
Created attachment 193119 [details]
Patch

Moved calculation of scaleFactor to a separate method where any ports that wishes different image-set behavior can east change it
Comment 9 Kenneth Rohde Christiansen 2013-03-22 06:48:04 PDT
(In reply to comment #8)
> Created an attachment (id=193119) [details]
> Patch
> 
> Moved calculation of scaleFactor to a separate method where any ports that wishes different image-set behavior can east change it

Shouldn't you make this opt in instead of opt out. Why not discuss this on www-style?
Comment 10 Allan Sandfeld Jensen 2013-03-22 07:05:53 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > Created an attachment (id=193119) [details] [details]
> > Patch
> > 
> > Moved calculation of scaleFactor to a separate method where any ports that wishes different image-set behavior can east change it
> 
> Shouldn't you make this opt in instead of opt out. Why not discuss this on www-style?

Well, it already is discussed and image-set is meant to take all scaling into account. This only allows for ports that wants to NOT follow the specification.
Comment 11 Beth Dakin 2013-03-22 11:35:40 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Created an attachment (id=193119) [details] [details] [details]
> > > Patch
> > > 
> > > Moved calculation of scaleFactor to a separate method where any ports that wishes different image-set behavior can east change it
> > 
> > Shouldn't you make this opt in instead of opt out. Why not discuss this on www-style?
> 
> Well, it already is discussed and image-set is meant to take all scaling into account. This only allows for ports that wants to NOT follow the specification.

But realistically, opting into this all the time could be a burden for a slow device on a slow network. Also, are you seeing flashes when the transition to the larger image happens? If the larger image takes a while to download or decode, we need a seamless transition between the two.
Comment 12 Allan Sandfeld Jensen 2013-03-22 12:13:29 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (In reply to comment #8)
> > > > Created an attachment (id=193119) [details] [details] [details] [details]
> > > > Patch
> > > > 
> > > > Moved calculation of scaleFactor to a separate method where any ports that wishes different image-set behavior can east change it
> > > 
> > > Shouldn't you make this opt in instead of opt out. Why not discuss this on www-style?
> > 
> > Well, it already is discussed and image-set is meant to take all scaling into account. This only allows for ports that wants to NOT follow the specification.
> 
> But realistically, opting into this all the time could be a burden for a slow device on a slow network. Also, are you seeing flashes when the transition to the larger image happens? If the larger image takes a while to download or decode, we need a seamless transition between the two.

That is might happen. You often see a flash anyway because pageZoom does a full relayout. The potential delay is also why I am not including other types of scaling yet. PageZoom is essentially only used by desktop computers when doing old CTRL+Plus scaling.

Ideally we should move resolving the image-set higher up. You could also imagine getting a timeout on one image and need to fetch the next best options, or that you still have the old one and the new one isn't ready. This is however bigger refactoring. This was just the easiest part that is still useful.
Comment 13 Andreas Kling 2014-02-05 11:20:16 PST
Comment on attachment 193119 [details]
Patch

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.