Last Comment Bug 755031 - Fix some build warnings under gfx/
: Fix some build warnings under gfx/
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla21
Assigned To: filidautore
:
Mentors:
Depends on: 833037 797666
Blocks: buildwarning 835625
  Show dependency treegraph
 
Reported: 2012-05-14 14:22 PDT by Josh Matthews [:jdm] (away until 9/3)
Modified: 2013-01-28 17:11 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
<required> (10.05 KB, patch)
2012-05-14 14:22 PDT, Josh Matthews [:jdm] (away until 9/3)
jmuizelaar: review+
Details | Diff | Splinter Review
Bug 755031 - Part 1: fix some warnings under gfx (28.85 KB, patch)
2013-01-09 09:05 PST, filidautore
jmuizelaar: review-
Details | Diff | Splinter Review
Bug 755031 - Clear warnings in gfx_2d_ScaledFontDWrite.cpp (2.19 KB, patch)
2013-01-15 07:21 PST, filidautore
jmuizelaar: review-
Details | Diff | Splinter Review
Bug 755031 - Clear warnings in gfx/2d/ScaledFontDWrite.cpp - v2 (2.66 KB, patch)
2013-01-15 12:39 PST, filidautore
jmuizelaar: review+
ryanvm: checkin+
Details | Diff | Splinter Review
Bug 755031 - Fix some other warnings in gfx/2d - v1 (3.89 KB, patch)
2013-01-16 06:11 PST, filidautore
jmuizelaar: feedback+
Details | Diff | Splinter Review
Bug 755031 - Clear warnings in gfx/gl - v1 (8.08 KB, patch)
2013-01-17 07:50 PST, filidautore
no flags Details | Diff | Splinter Review
Bug 755031 - Clear warnings in gfx/gl - v1.1 (1.87 KB, patch)
2013-01-18 05:50 PST, filidautore
jmuizelaar: review+
ryanvm: checkin+
Details | Diff | Splinter Review
Bug 755031 - Fix some other warnings in gfx/2d - v1.1 (3.94 KB, patch)
2013-01-18 08:29 PST, filidautore
jmuizelaar: review+
Details | Diff | Splinter Review
Bug 755031 - Fix some warnings in gfx/src - v1.0 (4.46 KB, patch)
2013-01-21 05:02 PST, filidautore
no flags Details | Diff | Splinter Review
Bug 755031 - Fix some other warnings in gfx/2d - v1.2 (3.16 KB, patch)
2013-01-28 02:28 PST, filidautore
filidautore: review+
ryanvm: checkin+
Details | Diff | Splinter Review

Description Josh Matthews [:jdm] (away until 9/3) 2012-05-14 14:22:34 PDT

    
Comment 1 Josh Matthews [:jdm] (away until 9/3) 2012-05-14 14:22:40 PDT
Created attachment 623808 [details] [diff] [review]
<required>
Comment 2 Jeff Muizelaar [:jrmuizel] 2012-05-17 12:58:11 PDT
Comment on attachment 623808 [details] [diff] [review]
<required>

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

The initializer list ordering is r+. The static_cast stuff is r- and needs more work.

::: gfx/layers/TiledLayerBuffer.h
@@ +338,1 @@
>        NS_ABORT_IF_FALSE(index >= 0 && index < newRetainedTiles.Length(), "index out of range");

I think this might be wrong. Send this hunk over to BenWa. Certainly the assertion below needs adjusting.

::: gfx/layers/opengl/TiledThebesLayerOGL.cpp
@@ +249,3 @@
>        uint16_t tileStartY = y % mVideoMemoryTiledBuffer.GetTileLength();
>        uint16_t h = mVideoMemoryTiledBuffer.GetTileLength() - tileStartY;
> +      if (y + h > static_cast<size_t>(visibleRect.y + visibleRect.height))

This makes the code worse to read. I think we need to come up with a better alternative.

::: gfx/thebes/nsCoreAnimationSupport.mm
@@ +475,5 @@
>    if (aWidth == 0 || aHeight == 0)
>      return NS_ERROR_FAILURE;
>  
> +  if (static_cast<uint32_t>(aWidth) == mUnsupportedWidth &&
> +      static_cast<uint32_t>(aHeight) == mUnsupportedHeight) {

Seems like the type should match instead of casting.
Comment 3 Daniel Holbert [:dholbert] 2012-08-31 12:29:09 PDT
Looks like this dropped off the radar for a few months.  The TiledLayerBuffer.h warning is still present (and spammy -- it's in a header file, and it gets triggered for each .cpp file that includes it).

jdm, are you still working on this?
Comment 4 Josh Matthews [:jdm] (away until 9/3) 2012-08-31 12:34:08 PDT
Not unless I get bored one of these days.
Comment 5 Daniel Holbert [:dholbert] 2012-10-03 22:34:44 PDT
FWIW, I just landed a fix for the TiledLayerBuffer.h one over in bug 797666.
Comment 6 filidautore 2013-01-09 09:05:31 PST
Created attachment 699886 [details] [diff] [review]
Bug 755031 - Part 1: fix some warnings under gfx
Comment 7 Jeff Muizelaar [:jrmuizel] 2013-01-14 12:12:07 PST
Comment on attachment 699886 [details] [diff] [review]
Bug 755031 - Part 1: fix some warnings under gfx

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

I don't really like all the casts that this patch adds. One suggestion would be to break the patch up into different parts that each address each type of warning. That will make it easier to review and the less controversial parts can go in first.

::: gfx/2d/ScaledFontDWrite.cpp
@@ +269,5 @@
>    if (fileOffset + fragmentSize > (UINT64)mData.size()) {
>      return E_FAIL;
>    }
>    // We should be alive for the duration of this.
> +  *fragmentStart = &mData[static_cast<unsigned int>(fileOffset)];

Why is this needed?
Comment 8 Daniel Holbert [:dholbert] 2013-01-14 12:38:33 PST
(In reply to Jeff Muizelaar [:jrmuizel] from comment #7)
> I don't really like all the casts that this patch adds.

Yeah -- as a build-warning-fixing crusader, my philosophy is that casts are generally evil & should only be used as a last resort.  In cases where they are actually necessary, they should be accompanied by a range-check to be sure the cast is valid, or an assertion and/or comment explaining why the value is already known to be in-range for the casted-to type.

> One suggestion would
> be to break the patch up into different parts that each address each type of
> warning. That will make it easier to review and the less controversial parts
> can go in first.

(+1 to this as well)
Comment 9 filidautore 2013-01-15 07:19:31 PST
Sorry for the previous patch, it became bigger than what I thought and then I decided to attach it... I'll try to split it in smaller parts.

(In reply to Jeff Muizelaar [:jrmuizel] from comment #7)
That cast is needed for clear this warning (probably there is a better fix for it):
warning C4244: 'argument' : conversion from 'UINT64' to 'unsigned int', possible loss of data

In that patch there were fixes for this kind of warning:
warning C4355: 'this' : used in base member initializer list
This warning will be cleared in Bug 826959
Comment 10 filidautore 2013-01-15 07:21:24 PST
Created attachment 702306 [details] [diff] [review]
Bug 755031 - Clear warnings in gfx_2d_ScaledFontDWrite.cpp

I hope this is better.
Comment 11 Jeff Muizelaar [:jrmuizel] 2013-01-15 11:21:02 PST
Comment on attachment 702306 [details] [diff] [review]
Bug 755031 - Clear warnings in gfx_2d_ScaledFontDWrite.cpp

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

::: gfx/2d/ScaledFontDWrite.cpp
@@ +269,5 @@
>    if (fileOffset + fragmentSize > (UINT64)mData.size()) {
>      return E_FAIL;
>    }
>    // We should be alive for the duration of this.
> +  *fragmentStart = &mData[static_cast<size_t>(fileOffset)];

I'd prefer something like this:

@@ -261,19 +261,23 @@ DWriteFontFileStream::GetLastWriteTime(UINT64 *lastWriteTime)
 
 HRESULT STDMETHODCALLTYPE
 DWriteFontFileStream::ReadFileFragment(const void **fragmentStart,
                                        UINT64 fileOffset,
                                        UINT64 fragmentSize,
                                        void **fragmentContext)
 {
   // We are required to do bounds checking.
-  if (fileOffset + fragmentSize > (UINT64)mData.size()) {
+  if (fileOffset + fragmentSize > mData.size()) {
     return E_FAIL;
   }
+
+  // truncate the 64 bit fileOffset to size_t sized index into mData
+  size_t index = static_cast<size_t>(fileOffset);
+
   // We should be alive for the duration of this.
   *fragmentStart = &mData[fileOffset];
   *fragmentContext = NULL;
   return S_OK;
 }
 
 void STDMETHODCALLTYPE
 DWriteFontFileStream::ReleaseFileFragment(void *fragmentContext)

@@ +389,5 @@
>    const void *fragmentStart;
>    void *context;
>    stream->ReadFileFragment(&fragmentStart, 0, fileSize, &context);
>  
> +  aDataCallback((uint8_t*)fragmentStart, (uint32_t)fileSize, mFontFace->GetIndex(), mSize, aBaton);

It might be better to give the filesize a new variable with the right type and do the truncation close to the check.
Comment 12 filidautore 2013-01-15 12:39:51 PST
Created attachment 702469 [details] [diff] [review]
Bug 755031 - Clear warnings in gfx/2d/ScaledFontDWrite.cpp - v2

Is this better?
Comment 13 Jeff Muizelaar [:jrmuizel] 2013-01-15 13:56:35 PST
Comment on attachment 702469 [details] [diff] [review]
Bug 755031 - Clear warnings in gfx/2d/ScaledFontDWrite.cpp - v2

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

Yep
Comment 14 Masatoshi Kimura [:emk] 2013-01-15 22:37:26 PST
>+private:
>+  BasicContainerLayer* This() { return this; }
Use ALLOW_THIS_IN_INITIALIZER_LIST macro instead of adding the method. See bug 827032 for details.
Comment 15 Daniel Holbert [:dholbert] 2013-01-15 23:02:46 PST
In particular, see bug 827032 comment 8 (which mentions the #include that you need -- "base/compiler_specific.h")
Comment 16 filidautore 2013-01-16 01:56:24 PST
(In reply to Masatoshi Kimura [:emk] from comment #14)
I thought Bug 826959 will fix that warning globally. I misread it?
If wanted I can fix it.
Comment 17 filidautore 2013-01-16 06:11:12 PST
Created attachment 702795 [details] [diff] [review]
Bug 755031 - Fix some other warnings in gfx/2d - v1

Fixed some warnings C4244 (possible loss of data).
I can't find a better way than using casts. Is there one?
Comment 18 Daniel Holbert [:dholbert] 2013-01-16 18:29:51 PST
Comment on attachment 702795 [details] [diff] [review]
Bug 755031 - Fix some other warnings in gfx/2d - v1

(In reply to filidautore from comment #17)
>diff --git a/gfx/2d/SourceSurfaceD2D.cpp b/gfx/2d/SourceSurfaceD2D.cpp
>--- a/gfx/2d/SourceSurfaceD2D.cpp
>+++ b/gfx/2d/SourceSurfaceD2D.cpp
>@@ -151,32 +151,34 @@ DataSourceSurfaceD2D::DataSourceSurfaceD
>-  D2D1_RENDER_TARGET_PROPERTIES rtProps = D2D1::RenderTargetProperties(
>-            D2D1_RENDER_TARGET_TYPE_DEFAULT,
>+  D2D1_RENDER_TARGET_PROPERTIES rtProps = D2D1::RenderTargetProperties(
>+            D2D1_RENDER_TARGET_TYPE_DEFAULT,
>             D2D1::PixelFormat(DXGI_FORMAT_UNKNOWN, D2D1_ALPHA_MODE_PREMULTIPLIED));
> 
>   RefPtr<ID2D1RenderTarget> renderTarget;
>-  hr = DrawTargetD2D::factory()->CreateDxgiSurfaceRenderTarget(dxgiSurface,
>-                                                               &rtProps,
>+  hr = DrawTargetD2D::factory()->CreateDxgiSurfaceRenderTarget(dxgiSurface,
>+                                                               &rtProps,

I don't see anything that changed between the removed and added lines, in the chunks quoted above. Am I missing something?  What's the patch doing here?
Comment 19 filidautore 2013-01-17 01:00:27 PST
(In reply to Daniel Holbert [:dholbert] from comment #18)
It changes the line endings from CRLF to LF.
Comment 20 filidautore 2013-01-17 07:50:46 PST
Created attachment 703320 [details] [diff] [review]
Bug 755031 - Clear warnings in gfx/gl - v1

This clears the warnings in gfx/gl:
- warning C4244 (possible loss of data)
  Appended |f| suffix to confirm the numbers are float.
- warning C4482: nonstandard extension used: enum 'xyz' used in qualified name
  Removed the name of the enum where used.
Comment 21 Masatoshi Kimura [:emk] 2013-01-17 07:58:42 PST
(In reply to filidautore from comment #20)
> - warning C4482: nonstandard extension used: enum 'xyz' used in qualified
> name
>   Removed the name of the enum where used.

Just disable this warning. It becomes a standard since C++11, and MSVC11 removed the warning.
Comment 22 filidautore 2013-01-17 08:24:42 PST
(In reply to Masatoshi Kimura [:emk] from comment #21)
How I do it? Is there some examples?
Comment 23 Masatoshi Kimura [:emk] 2013-01-17 14:01:54 PST
#pragma warning(disable : 4482)
or even adding -wd4482 to configure.in (see bug 826611).
Comment 24 Daniel Holbert [:dholbert] 2013-01-17 14:04:12 PST
(In reply to Masatoshi Kimura [:emk] from comment #21)
> (In reply to filidautore from comment #20)
> > - warning C4482: nonstandard extension used: enum 'xyz' used in qualified
> > name
> Just disable this warning. It becomes a standard since C++11, and MSVC11
> removed the warning.

If you think we should do that tree-wide, that should probably be its own bug (and get review from a build peer), separate from this /gfx-specific stuff.
Comment 25 Daniel Holbert [:dholbert] 2013-01-17 14:15:23 PST
In this bug's specific case, there's a better way to address the C4482 enum-namespacing warnings -- just upgrade SharedHandleType to an enum class, instead of "typedef { ...} enum".

That will give us better type-safety, so it's a worthwhile change even if we don't care about (and end up disabling) the warning.  And it'd silence the warning, because enum class values are *supposed* to be namespaced, so after this change, all of the "SharedHandleType::" prefixes would become correct.

For reference, see this file for documentation:
 https://mxr.mozilla.org/mozilla-central/source/mfbt/Attributes.h#355
and this chunk of code for an example in action:
 https://mxr.mozilla.org/mozilla-central/source/editor/libeditor/base/nsEditor.h#89
Comment 26 Jonathan Kew (:jfkthame) 2013-01-17 15:23:49 PST
Comment on attachment 702795 [details] [diff] [review]
Bug 755031 - Fix some other warnings in gfx/2d - v1

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

::: gfx/2d/RecordedEvent.cpp
@@ +200,5 @@
>  
>  void
>  RecordedEvent::ReadStrokeOptions(std::istream &aStream, StrokeOptions &aStrokeOptions)
>  {
> +  size_t dashLength;

This doesn't look right, as the field was explicitly written as uint64_t (why? I'm not sure) in the RecordStrokeOptions function just above...

@@ +205,4 @@
>    JoinStyle joinStyle;
>    CapStyle capStyle;
>  
>    ReadElement(aStream, dashLength);

...which means this Read won't match it's corresponding Write, unless that is also changed.
Comment 27 filidautore 2013-01-18 05:08:09 PST
(In reply to Masatoshi Kimura [:emk] from comment #23)
I filed Bug 832280.
Comment 28 filidautore 2013-01-18 05:50:47 PST
Created attachment 703882 [details] [diff] [review]
Bug 755031 - Clear warnings in gfx/gl - v1.1

Added suffix |f| to the numbers to confirm they are float.
Comment 29 filidautore 2013-01-18 08:29:11 PST
Created attachment 703917 [details] [diff] [review]
Bug 755031 - Fix some other warnings in gfx/2d - v1.1

Same patch as before where I fixed some warnings C4244 (possible loss of data) but instead of change the type of |dashLength| I added another cast.
I don't understand well the reason but, seeing bug 811850, seems I can't change the type to size_t in ReadElement/WriteElement methods.
Comment 30 filidautore 2013-01-21 05:02:47 PST
Created attachment 704520 [details] [diff] [review]
Bug 755031 - Fix some warnings in gfx/src - v1.0

I added some simple methods with the right return type.
I added a cast to prefDPI because I wasn't able to change Preferences::GetInt to Preferences::GetFloat without triggering an error.
Comment 31 Masatoshi Kimura [:emk] 2013-01-21 06:52:56 PST
Comment on attachment 704520 [details] [diff] [review]
Bug 755031 - Fix some warnings in gfx/src - v1.0

>+    float AppUnitsPerDevPixelF() const { return float(mAppUnitsPerDevPixel); }
Why don't you change the type of mAppUnitsPerDevPixel to float?
Comment 32 Daniel Holbert [:dholbert] 2013-01-21 09:48:28 PST
(In reply to Masatoshi Kimura [:emk] from comment #31)
> >+    float AppUnitsPerDevPixelF() const { return float(mAppUnitsPerDevPixel); }
> Why don't you change the type of mAppUnitsPerDevPixel to float?

I don't think that makes sense.  It's an integral value (normaly 60), and we use it in integer multiplication all over the place.  We don't need all of those places to suddenly be using float math, just because we use it as a float in a few places.
Comment 33 Masatoshi Kimura [:emk] 2013-01-21 09:58:00 PST
(In reply to Daniel Holbert [:dholbert] from comment #32)
> (In reply to Masatoshi Kimura [:emk] from comment #31)
> > >+    float AppUnitsPerDevPixelF() const { return float(mAppUnitsPerDevPixel); }
> > Why don't you change the type of mAppUnitsPerDevPixel to float?
> 
> I don't think that makes sense.  It's an integral value (normaly 60), and we
> use it in integer multiplication all over the place.  We don't need all of
> those places to suddenly be using float math, just because we use it as a
> float in a few places.

Actually it will be converted to float or double in most cases except NSIntPixelsToAppUnits.
https://mxr.mozilla.org/mozilla-central/source/gfx/src/nsCoord.h#342
NSIntPixelsToAppUnits casts it to float explicitly (so float math will not be performed here even when appUnitsPerPixels is converted to float).
Thus we are doing float->int->float conversions at present. It should be float from the start to avoid the extra conversion, IMO.
Comment 34 Daniel Holbert [:dholbert] 2013-01-21 10:09:23 PST
> Actually it will be converted to float or double in most cases except NSIntPixelsToAppUnits.

I'm not sure I buy that. It's used as an integer via these PresContext methods:
 https://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.h#547
which are called all over the place, and which I've absolutely seen used in int math. (in gfxFont.cpp recently, via a few levels of indirection)

In any case -- if we we decided to change that, we'd want to change all of those methods (including the PresContext wrappers), and that would absolutely merit its own bug -- it shouldn't be buried N patches and M comments into a large generic "fix build warnings in /dir" bug.  So, this is not the right place to do that change.
Comment 35 Daniel Holbert [:dholbert] 2013-01-21 10:10:12 PST
filidautore: if you have any additional patches, could you file separate bugs for them? This bug here is getting pretty hard-to-follow, with various patches for different subdirs being posted & discussed & rehashed & split up & obsoleted.

Bugs are cheap, and where possible, it's best to have individual bugs track small, bite-sized issues.

Clarifying bug summary (s/fix build warnings/fix some build warnings/) to make this bug less monolithic.
Comment 36 Masatoshi Kimura [:emk] 2013-01-21 10:26:13 PST
(In reply to Daniel Holbert [:dholbert] from comment #34)
> > Actually it will be converted to float or double in most cases except NSIntPixelsToAppUnits.
> 
> I'm not sure I buy that. It's used as an integer via these PresContext
> methods:
>  https://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.
> h#547
> which are called all over the place, and which I've absolutely seen used in
> int math. (in gfxFont.cpp recently, via a few levels of indirection)

I couldn't find integer arithmetic of mAppUnitsPerDevUnit in gfxFont.cpp. Where did you find it?

>        gfxFloat width = std::max(aFont->GetMetrics().aveCharWidth,
>                                gfxFontMissingGlyphs::GetDesiredMinWidth(aChar));
>        details->mAdvance = uint32_t(width * mAppUnitsPerDevUnit);
This is converted to double (a.k.a. gfxFloat) because width is gfxFloat.

>gfxShapedText::AdjustAdvancesForSyntheticBold(float aSynBoldOffset,
>                                              uint32_t aOffset,
>                                              uint32_t aLength)
>{
>    uint32_t synAppUnitOffset = aSynBoldOffset * mAppUnitsPerDevUnit;
This is converted to float because aSynBoldOffset is float.

>    gfxRect clipExtents = aCtx->GetClipExtents();
>    gfxFloat left = clipExtents.X()*mAppUnitsPerDevUnit;
>    gfxFloat right = clipExtents.XMost()*mAppUnitsPerDevUnit;
These are converted to double because gfxRect uses gfxFloat dimensions.

>    gfxFloat left = clipExtents.X()*mAppUnitsPerDevUnit;
>    gfxFloat right = clipExtents.XMost()*mAppUnitsPerDevUnit;
(snip)
>      aCtx->Rectangle(gfxRect(left / mAppUnitsPerDevUnit,
>                              clipExtents.Y(),
>                              (right - left) / mAppUnitsPerDevUnit,
>                              clipExtents.Height()), true);
These are converted to double because left and right are gfxFloat.

>    uint32_t spaceWidthAppUnits =
>        NS_lroundf(aFont->GetMetrics().spaceWidth * mAppUnitsPerDevUnit);
This is converted to double because spaceWidth is gfxFloat.

I was working on enabling FAIL_ON_WARNINGS on MSVC under gfx/, and found that floating-point arithmetics were more frequent than integer arithmetics for appUnitsPerPicels. But I suspended the work because the patch became too large.

> In any case -- if we we decided to change that, we'd want to change all of
> those methods (including the PresContext wrappers), and that would
> absolutely merit its own bug -- it shouldn't be buried N patches and M
> comments into a large generic "fix build warnings in /dir" bug.  So, this is
> not the right place to do that change.

I agree about that.
Comment 37 Daniel Holbert [:dholbert] 2013-01-21 10:33:46 PST
I was talking about usages of AppUnitsPerDevPixel() and related methods, not the mAppUnitsPerDevUnit variable itself.  (since that's the primary way that mAppUnitsPerDevUnit ends up being usd)
Comment 38 Daniel Holbert [:dholbert] 2013-01-21 10:39:54 PST
(oh, sorry, I missed the "in gfxFont.cpp" in your last comment -- I thought those were all usages of the nsDeviceContext variable.  My bad)

I'm pretty sure these lines in gfxFont use int math:
> 2258     metrics.mAscent = fontMetrics.maxAscent*appUnitsPerDevUnit;
> 2259     metrics.mDescent = fontMetrics.maxDescent*appUnitsPerDevUnit;
https://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxFont.cpp#2258

But anyway, let's not discuss this further here. If you really we should tweak that, please file a followup bug, and we can discuss it there. :)
Comment 39 Masatoshi Kimura [:emk] 2013-01-21 10:54:57 PST
(In reply to Daniel Holbert [:dholbert] from comment #38)
> But anyway, let's not discuss this further here. If you really we should
> tweak that, please file a followup bug, and we can discuss it there. :)

OK, filed bug 833037.
Comment 40 Jeff Muizelaar [:jrmuizel] 2013-01-25 11:18:10 PST
Comment on attachment 703917 [details] [diff] [review]
Bug 755031 - Fix some other warnings in gfx/2d - v1.1

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

::: gfx/2d/RecordedEvent.cpp
@@ +210,5 @@
>    ReadElement(aStream, aStrokeOptions.mLineWidth);
>    ReadElement(aStream, aStrokeOptions.mMiterLimit);
>    ReadElement(aStream, joinStyle);
>    ReadElement(aStream, capStyle);
> +  aStrokeOptions.mDashLength = size_t(dashLength);

Add a comment that we're truncating dashLength on 32 bit
Comment 41 filidautore 2013-01-28 02:14:43 PST
Comment on attachment 704520 [details] [diff] [review]
Bug 755031 - Fix some warnings in gfx/src - v1.0

Part of this bug will be fixed by bug 833037 or bug 832440.
Comment 42 filidautore 2013-01-28 02:28:00 PST
Created attachment 706995 [details] [diff] [review]
Bug 755031 - Fix some other warnings in gfx/2d - v1.2

I added the comment. I hope is right.
Comment 43 filidautore 2013-01-28 02:35:53 PST
Can someone land this patches and close the bug?
Comment 44 Ryan VanderMeulen [:RyanVM] 2013-01-28 07:10:05 PST
Comment on attachment 702469 [details] [diff] [review]
Bug 755031 - Clear warnings in gfx/2d/ScaledFontDWrite.cpp - v2

https://hg.mozilla.org/integration/mozilla-inbound/rev/15a439da839d
Comment 45 Ryan VanderMeulen [:RyanVM] 2013-01-28 07:10:15 PST
Comment on attachment 703882 [details] [diff] [review]
Bug 755031 - Clear warnings in gfx/gl - v1.1

https://hg.mozilla.org/integration/mozilla-inbound/rev/6565fc57003d
Comment 46 Ryan VanderMeulen [:RyanVM] 2013-01-28 07:10:27 PST
Comment on attachment 706995 [details] [diff] [review]
Bug 755031 - Fix some other warnings in gfx/2d - v1.2

https://hg.mozilla.org/integration/mozilla-inbound/rev/15500baebbc5
Comment 47 Ryan VanderMeulen [:RyanVM] 2013-01-28 07:10:50 PST
Thanks for the patches!

Note You need to log in before you can comment on or make changes to this bug.