ASSERTION: Invalid (negative) scale factor: 'sy >= 0.0'

RESOLVED FIXED in mozilla25

Status

()

Core
Graphics: Layers
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: romaxa, Assigned: romaxa)

Tracking

unspecified
mozilla25
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

4 years ago
BasicTiledLayerBuffer::ComputeProgressiveUpdateRegion

declare  float scaleX, scaleY
call if (mManager->ProgressiveUpdateCallback
http://mxr.mozilla.org/mozilla-central/source/gfx/layers/client/TiledContentClient.cpp#317

which is defined only for android: 
http://mxr.mozilla.org/mozilla-central/source/gfx/layers/client/ClientLayerManager.cpp#397

as result we have undefined scaleX, scaleY
in undefined state
(Assignee)

Comment 1

4 years ago
Created attachment 773019 [details] [diff] [review]
Possible fix
Attachment #773019 - Flags: review?(ajones)
(Assignee)

Comment 2

4 years ago
Created attachment 773020 [details] [diff] [review]
Possible fix
Attachment #773019 - Attachment is obsolete: true
Attachment #773019 - Flags: review?(ajones)
Attachment #773020 - Flags: review?(ajones)
Comment on attachment 773020 [details] [diff] [review]
Possible fix

Looks OK to me but Nick might like to review this one.
Attachment #773020 - Flags: review?(ajones) → review?(ncameron)

Comment 4

4 years ago
I would prefer ProgressiveUpdateCallback to set the scales to 1.0 if we are not on Android so that it has a platform-specific 'contract'. I'd be happy to have scaleX and scaleY initialised as well, just to be on the safe side, but it should not be necessary.

Updated

4 years ago
Attachment #773020 - Flags: review?(ncameron)
(Assignee)

Comment 5

4 years ago
Created attachment 774392 [details] [diff] [review]
Fixed review comment
Attachment #773020 - Attachment is obsolete: true
Attachment #774392 - Flags: review?(ncameron)

Comment 6

4 years ago
Comment on attachment 774392 [details] [diff] [review]
Fixed review comment

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

::: gfx/layers/client/ClientLayerManager.cpp
@@ +419,5 @@
>        aHasPendingNewThebesContent, displayPort, paintScale.scale, aDrawingCritical,
>        aViewport, aScaleX, aScaleY);
>    }
> +#else
> +  aScaleX = aScaleY = 1.0;

Should we do this if we are on Android but don't go through the if clause? It seems like we should get a similar situation in the caller.
(Assignee)

Comment 7

4 years ago
would it make sense just initialize aScaleX = aScaleY = 1.0; at the beginning of ProgressiveUpdateCallback function and don't bother if primaryScrollable case or other if's inside AndroidBridge::Bridge()->ProgressiveUpdateCallback will fail?

Comment 8

4 years ago
(In reply to Oleg Romashin (:romaxa) from comment #7)
> would it make sense just initialize aScaleX = aScaleY = 1.0; at the
> beginning of ProgressiveUpdateCallback function and don't bother if
> primaryScrollable case or other if's inside
> AndroidBridge::Bridge()->ProgressiveUpdateCallback will fail?

Yes, I think that makes sense.
(Assignee)

Comment 9

4 years ago
Created attachment 774446 [details] [diff] [review]
Init scale in ProgressiveUpdateCallback
Attachment #774392 - Attachment is obsolete: true
Attachment #774392 - Flags: review?(ncameron)
Attachment #774446 - Flags: review?(ncameron)

Updated

4 years ago
Attachment #774446 - Flags: review?(ncameron) → review+
(Assignee)

Updated

4 years ago
Attachment #774446 - Flags: checkin?
Assignee: nobody → romaxa
Status: NEW → ASSIGNED
Attachment #774446 - Flags: checkin? → checkin+
https://hg.mozilla.org/integration/mozilla-inbound/rev/84df0a7f8ee6
https://hg.mozilla.org/mozilla-central/rev/84df0a7f8ee6
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.