967794 - GetLowPrecisionResolution not doing what it appears to want to do

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Milan Sreckovic [:milan] (needinfo for best results)]

gfxPlatform::GetLowPrecisionResolution() is written as if we want the latest value (e.g., if the preference is updated during the session, the new value is picked up), but it isn't actually doing that.

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Make sure that in-session changes to the preference are picked up.

It wasn't clear if we wanted to pick up the up to date version during the session, but the way it was written, I think that changing the preference during the session would do bad things or crash, as we're using local variable in the callback.

This change makes it so that we pick up the new value - if we want to just do the "once per session", we want to get rid of the callback.

Comment 2

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

Huh, I'm pretty sure vlad brought this code up on IRC at some point and how it was broken. Guess he never actually fixed it?

Comment 3

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

Yeah... http://logbot.glob.com.au/?c=mozilla%23gfx&s=18+Nov+2013&e=18+Nov+2013#c108896

Comment 4

[Chris Lord [:cwiiis]]

Comment on attachment 8370303 [details] [diff] [review]
Make sure that in-session changes to the preference are picked up.

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

r+ with the changes below. Sorry for messing this up in the first place :)

::: gfx/thebes/gfxPlatform.cpp
@@ +1610,5 @@
>  
>  float
>  gfxPlatform::GetLowPrecisionResolution()
>  {
> +    static int32_t sLowPrecisionResolutionX1000;

Let's put the = 250 here and remove line 1618 (sLowPrecisionResolutionX1000 = 250;)

@@ +1621,2 @@
>                                               "layers.low-precision-resolution",
>                                               250);

with the above change, the 250 here should be sLowPrecisionResolutionX1000 so we don't have the 250 in more than one place.

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Make sure that in-session changes to the preference are picked up. With review comments addressed. Carry r=clord.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3515cd83e2aa

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3515cd83e2aa