Bug 111304 – -webkit-image-set should consider Frame::pageZoomFactor()

• Component: CSS
• Status: NEW

[Allan Sandfeld Jensen]

-webkit-image-set currently only considers deviceScaleFactor, if it also supported pageZoomFactor it would mean we could pick higher resolution images on zoomed pages.

[Allan Sandfeld Jensen]

Created attachment 191211 [details]
Patch

[Kenneth Rohde Christiansen]

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?

[Allan Sandfeld Jensen]

Created attachment 191218 [details]
Patch

Now with a test

[Allan Sandfeld Jensen]

(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.

[Allan Sandfeld Jensen]

Created attachment 191231 [details]
Patch

Avoid rerequesting images when the scale changes in a way that does not affect the choice of image

[Beth Dakin]

I definitely think this feature is interesting, but it should be opt-in somehow. All ports may not want to do this.

[Allan Sandfeld Jensen]

(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.

[Allan Sandfeld Jensen]

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

[Kenneth Rohde Christiansen]

(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?

[Allan Sandfeld Jensen]

(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.

[Beth Dakin]

(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.

[Allan Sandfeld Jensen]

(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.

[Andreas Kling]

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.