1008412 - With low-precision buffer enabled the displayport exceeds max texture size

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

When low-precision buffer is enabled with the default settings, the displayport ends up exceeding 4096 pixels in some cases. This causes us to trigger the DisplayportExceedsMaxTextureSize check in nsGfxScrollFrame, and switches back to sync scrolling.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Account for low-precision resolution too

This fixes the problem for me, thanks for the suggestion!

Comment 2

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8420361 [details] [diff] [review]
Account for low-precision resolution too

Presumably we want the same size limit on the "critical display port" size. But I think right now this will accomplish that since the size of the display port and critical display port are related by a scale factor of the low prec resolution. Are we planning on changing that relation?

Comment 3

[Chris Lord [:cwiiis]]

Comment on attachment 8420361 [details] [diff] [review]
Account for low-precision resolution too

Review of attachment 8420361 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsGfxScrollFrame.cpp
@@ +2416,5 @@
>    // Note also that if we don't do this here, it is quite likely that the parent B2G process
>    // will kill this child process to prevent OOMs (see the patch that landed as part of bug
>    // 965945 for details).
>    gfxSize resolution = aPresContext->PresShell()->GetCumulativeResolution();
> +  if (gfxPrefs::UseLowPrecisionBuffer()) {

I think this should also check that the critical displayport isn't empty? If we have low precision rendering enabled but an empty critical displayport, I think we just use high precision rendering in the displayport, iirc.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

(In reply to Timothy Nikkel (:tn) from comment #2)
> Presumably we want the same size limit on the "critical display port" size.
> But I think right now this will accomplish that since the size of the
> display port and critical display port are related by a scale factor of the
> low prec resolution. Are we planning on changing that relation?

We might change that relationship eventually, although I have no plans to do so in the immediate future. Still, it's a good idea to check for that.

(In reply to Chris Lord [:cwiiis] from comment #3)
> I think this should also check that the critical displayport isn't empty? If
> we have low precision rendering enabled but an empty critical displayport, I
> think we just use high precision rendering in the displayport, iirc.

With the patches on bug 1001438 this won't happen any more. Front-end code will only set a displayport and then low precision rendering is enabled it will automatically get used as  the critical display port, and the displayport will be a multiple of it. However since bug 1001438 is still pending review/test-fixing and I'd like to not have to wait on that, I'll add a check for this too.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Attachment: Account for low-precision resolution too (v2)

Updated patch that deals with both comments above.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

https://tbpl.mozilla.org/?tree=Try&rev=8a9bea7d42a6

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2ffd315c525f

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2ffd315c525f