Currently GraphicsLayerCA uses pageScaleFactor in requiresTiledLayer() and computePixelAlignment(), but we must use pageScaleFactor * deviceScaleFactor in them. It is because: 1. requiresTiledLayer() uses the scale to compute an actual layer size in the device pixel unit. 2. computePixelAlignment() uses the scale to compute an aligned layer position in the device pixel unit.
Created attachment 183603 [details] Patch
Comment on attachment 183603 [details] Patch Clearing flags on attachment: 183603 Committed r150047: <http://trac.webkit.org/changeset/150047>
All reviewed patches have been landed. Closing bug.
Comment on attachment 183603 [details] Patch This sis wrong. We used to do this, and it causes pages to have different behavior on Retina and non-retina devices, which is undesirable. We’re not required to fall into tiled layers for larger surfaces; it’s just a memory optimization. Please revert the change.
(In reply to comment #4) > (From update of attachment 183603 [details]) > This sis wrong. We used to do this, and it causes pages to have different behavior on Retina and non-retina devices, which is undesirable. We’re not required to fall into tiled layers for larger surfaces; it’s just a memory optimization. > > Please revert the change. Thank you for explanation. Can I ask a question?, because I don't fully catch up yet. In my understand of your explanation, it means that On Ratina, WebKit draws content on the same sized backing store and scales 2 times on the device. is it right? Let me say that the device scale factor on Ratina is 2. My curiosity is as follow: First of all, below code also causes different behavior on Retina and non-retina devices. afaik, layer's backing store on Ratina will be 2 times bigger than it on non-ratina, because contentsScale is 2 times bigger on Ratina. void GraphicsLayerCA::updateContentsScale(float pageScaleFactor) { float contentsScale = clampedContentsScaleForScale(pageScaleFactor * deviceScaleFactor()); m_layer->setContentsScale(contentsScale); if (drawsContent()) m_layer->setNeedsDisplay(); } In addition, backing store on Ratina should be two times bigger to draw text clearly. Even after rethinking, imo, my patch seems to be correct..
(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 183603 [details] [details]) > > This sis wrong. We used to do this, and it causes pages to have different behavior on Retina and non-retina devices, which is undesirable. We’re not required to fall into tiled layers for larger surfaces; it’s just a memory optimization. > > > > Please revert the change. > > Thank you for explanation. Can I ask a question?, because I don't fully catch up yet. > > In my understand of your explanation, it means that On Ratina, WebKit draws content on the same sized backing store and scales 2 times on the device. is it right? Correct. > Let me say that the device scale factor on Ratina is 2. > > My curiosity is as follow: > > First of all, below code also causes different behavior on Retina and non-retina devices. afaik, layer's backing store on Ratina will be 2 times bigger than it on non-ratina, because contentsScale is 2 times bigger on Ratina. Actually 4x in memory use. > void GraphicsLayerCA::updateContentsScale(float pageScaleFactor) > { > float contentsScale = clampedContentsScaleForScale(pageScaleFactor * deviceScaleFactor()); Right, this is clamping based on the estimated memory use for the layer. Actually that could be pageScaleFactor * deviceScaleFactor() * deviceScaleFactor(). > m_layer->setContentsScale(contentsScale); > if (drawsContent()) > m_layer->setNeedsDisplay(); > } > > In addition, backing store on Ratina should be two times bigger to draw text clearly. > > Even after rethinking, imo, my patch seems to be correct.. It is theoretically correct, but introduces unwanted behavior. I want the layer size threshold at which requiresTiledLayer() returns true to be the same for retina and non-retina devices. That does mean that we allow layer memory use on retina to be larger before we fall into tiled layers, but we're willing to live with that. I'm curious about what device you care about that's using GraphicsLayerCA?
(In reply to comment #6) > It is theoretically correct, but introduces unwanted behavior. I want the layer size threshold at which requiresTiledLayer() returns true to be the same for retina and non-retina devices. That does mean that we allow layer memory use on retina to be larger before we fall into tiled layers, but we're willing to live with that. I see. I'm ok to roll back this patch. I can not do it because I don't have committer permission. > I'm curious about what device you care about that's using GraphicsLayerCA? Nothing. On this January, when I read GraphicsLayerCA to understand original GraphicsLayer implementation, I just found the point to conflict with what I understood. I'm sorry for noise.
Reverted in https://trac.webkit.org/r150135