Compile time error for unused variable 'invmatrix' on nsSVGForeignObjectFrame

RESOLVED INVALID

Status

()

Core
Layout: Block and Inline
RESOLVED INVALID
6 years ago
6 years ago

People

(Reporter: sinker, Assigned: sinker)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
invmatrix is only used for assertion.  It dues to a compiling time error for unused variable.
(Assignee)

Comment 1

6 years ago
Created attachment 597638 [details] [diff] [review]
Fix the compile time error for unused variable invmatrix
Assignee: nobody → tlee
Status: NEW → ASSIGNED
Attachment #597638 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Blocks: 674728
Comment on attachment 597638 [details] [diff] [review]
Fix the compile time error for unused variable invmatrix

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

r+ with that

::: layout/svg/base/src/nsSVGForeignObjectFrame.cpp
@@ +273,5 @@
>    gfx->Multiply(matrixForChildren);
>  
>    // Transform the dirty rect into the rectangle containing the
>    // transformed dirty rect.
> +  mozilla::DebugOnly<gfxMatrix> invmatrix = matrix.Invert();

Instead of a mozilla:: prefix here, just add "using namespace mozilla;" after the #includes.
Attachment #597638 - Flags: review?(roc) → review+
(Assignee)

Comment 3

6 years ago
Created attachment 597650 [details] [diff] [review]
Fix the compile time error for unused variable invmatrix, v2

updated as the suggestion of comment 2.
Attachment #597638 - Attachment is obsolete: true

Comment 4

6 years ago
Try run for b6c7a39cd692 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=b6c7a39cd692
Results (out of 133 total builds):
    success: 110
    warnings: 10
    failure: 13
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/tlee@mozilla.com-b6c7a39cd692
(Assignee)

Comment 5

6 years ago
Created attachment 597712 [details] [diff] [review]
Fix the compile time error for unused variable invmatr ix, v3
Attachment #597650 - Attachment is obsolete: true
I don't like it. matrix.Invert() actually changes the matrix, unnecessarily in opt builds. I'd add a little helper

#ifdef DEBUG
const gfxMatrix&
Inverse(const gfxMatrix& aMatrix)
{
  gfxMatrix matrix = aMatrix;
  return matrix.Invert();
}
#endif

And then change the two assertions in this function by

NS_ASSERTION(!Inverse(matrix).IsSingular(),
             "inverse of non-singular matrix should be non-singular");

Or something like that
This code was removed in bug 727212
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → INVALID

Comment 8

6 years ago
Try run for c0cded1d82ce is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=c0cded1d82ce
Results (out of 212 total builds):
    success: 175
    warnings: 23
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/tlee@mozilla.com-c0cded1d82ce
You need to log in before you can comment on or make changes to this bug.