Bug 227614 – [CG] REGRESSION(r278863): The destination rectangle is truncated when the sub-image is used

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Said Abou-Hallawa]

This was overlooked when GraphicsContextCG::drawNativeImage() was re-factored in r278863.

[Said Abou-Hallawa]

<rdar://79840643>

[Said Abou-Hallawa]

Created attachment 432773 [details]
Patch

[Said Abou-Hallawa]

Created attachment 432827 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 432827 [details]
Patch

If we are drawing the whole image why do we hit the subimage codepath?

[Said Abou-Hallawa]

Consider the case of drawing the whole ImageBuffer through ImageBuffer::draw(). Assume this ImageBuffer has the following parameters:
    logicalSize = { 392, 44 }
    resolutionScale = 1.91326535

ImageBufferBackend::calculateBackendSize() gets the enclosing rectangle for the backend size:
    backendSize = ceil(logicalSize * resolutionScale) = ceil({ 392 * 1.91326535, 44 * 1.91326535 }) = ceil({ 750, 84.1836754 }) = { 750, 85 }

The caller will pass the following arguments to ImageBuffer::draw():
    destRect = { 0,  0, 392, 44 }
    srcRect = { 0, 0, 392, 44 }

But ImageBufferCGBackend::draw() will pass the following arguments to GraphicsContextCG::drawNativeImage()
    imageSize = { 750, 85 } // = backendSize
    destRect = { 0,  0, 392, 44 }
    srcRect = { 0, 0, 750, 84.1836754 } // = logicalSize * resolutionScale

The condition to use the subimage codepath is the following:

    if (normalizedSrcRect != imageRect) // normalizedSrcRect is equal to srcRect in this case.

And this condition is true in this case because { 0, 0, 750, 84.1836754 } != { 0, 0, 750, 85 }

So yes in this case, we should not hit the subimage codepath. But I was trying to fix the regression and to put the calculation code back as it was before r278863.

[Simon Fraser (smfr)]

Yes, my point was let's fix the rounding issues so we don't hit the subimage code path  for every back buffer -> front buffer copy which will never benefit from the cache.

[Said Abou-Hallawa]

Created attachment 432978 [details]
Patch

[Said Abou-Hallawa]

I fixed the issue of drawing the back ImageBuffer to the front ImageBuffer in RemoteLayerBackingStore::display(). Drawing the whole ImageBuffer should not go through the sub-image code path. But I had to adjust the calculation of the imageSize to match the logicalSize and the destRect to match the backendSize correctly.

[Said Abou-Hallawa]

I think this bug is related to bug 225377

[Said Abou-Hallawa]

Created attachment 433074 [details]
Patch

[Said Abou-Hallawa]

Created attachment 433394 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 433394 [details]
Patch

Since this restores the behavior to shipping, r=me. However, we need to figure out how to avoid the sub image code path here (bug 227912).

[EWS]

Committed r279885 (239637@main): <https://commits.webkit.org/239637@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433394 [details].