Closed Bug 959380 Opened 6 years ago Closed 6 years ago

Convert thebes/gfxTypes.h to typed enums

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: bjacob, Assigned: bjacob)

Details

(Whiteboard: [qa-])

Attachments

(12 files)

10.45 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
1.77 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
172.31 KB, patch
jrmuizel
: feedback+
Details | Diff | Splinter Review
8.97 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
2.09 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
44.57 KB, patch
jrmuizel
: feedback+
Details | Diff | Splinter Review
2.99 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
2.03 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
90.76 KB, patch
jrmuizel
: feedback+
Details | Diff | Splinter Review
3.58 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
1.15 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
5.60 KB, patch
jrmuizel
: feedback+
Details | Diff | Splinter Review
No description provided.
Small one, all manual changes.
Attachment #8359442 - Flags: review?(jmuizelaar)
find . -type f | grep -v \./obj | grep -v \.hg | xargs sed -i 's/\(^\|[^A-Za-z0-9_]\)gfxImageFormat\(ARGB32\|RGB24\|A8\|A1\|RGB16_565\|Unknown\)\($\|[^A-Za-z0-9_]\)/\1gfxImageFormat::\2\3/g'
Attachment #8359447 - Flags: feedback?(jmuizelaar)
Note: for each X, the set of patches { Part X.Y, for all Y } will be rolled into one cset for landing, so as to land a patch set where each step is green.
Attachment #8359453 - Flags: review?(jmuizelaar) → feedback?(jmuizelaar)
Btw part 3.2 was generated by:

find . -type f | grep -v \./obj | grep -v \.hg | xargs sed -i 's/\(^\|[^A-Za-z0-9_]\)gfxSurfaceType\(Image\|PDF\|PS\|Xlib\|Xcb\|Glitz\|Quartz\|Win32\|BeOS\|DirectFB\|SVG\|OS2\|Win32Printing\|QuartzImage\|Script\|QPainter\|Recording\|VG\|GL\|DRM\|Tee\|XML\|Skia\|Subsurface\|D2D\|Max\)\($\|[^A-Za-z0-9_]\)/\1gfxSurfaceType::\2\3/g'
find . -type f | grep -v \./obj | grep -v \.hg | xargs sed -i 's/\(^\|[^A-Za-z0-9_]\)GFX_CONTENT_\(COLOR\|ALPHA\|COLOR_ALPHA\|SENTINEL\)\($\|[^A-Za-z0-9_]\)/\1gfxContentType::\2\3/g'
Attachment #8359459 - Flags: feedback?(jmuizelaar)
gfxMemoryLocation will only get 2 patches, since the initial manual changes here, plus the automatic changes in the next patch, are all what turned out to be needed.
Attachment #8359463 - Flags: review?(jmuizelaar)
find . -type f | grep -v \./obj | grep -v \.hg | xargs sed -i 's/\(^\|[^A-Za-z0-9_]\)GFX_MEMORY_\(IN_PROCESS_HEAP\|IN_PROCESS_NONHEAP\|OUT_OF_PROCESS\)\($\|[^A-Za-z0-9_]\)/\1gfxMemoryLocation::\2\3/g'
Attachment #8359464 - Flags: feedback?(jmuizelaar)
Ready for review; a previous try push was all green, but this bitrots very fast, so here's a new rebased try:

https://tbpl.mozilla.org/?tree=Try&rev=cea61fa7a1ba
Comment on attachment 8359442 [details] [diff] [review]
Part 1: gfxBreakPriority

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

::: gfx/thebes/gfxTypes.h
@@ +38,5 @@
>   *
>   * @see gfxTextRun::BreakAndMeasureText
>   * @see nsLineLayout::NotifyOptionalBreakPosition
>   */
> +MOZ_BEGIN_ENUM_CLASS(gfxBreakPriority, int8_t)

I'd rather not have the explicit type here.
Attachment #8359442 - Flags: review?(jmuizelaar) → review+
Attachment #8359443 - Flags: review?(jmuizelaar) → review+
Attachment #8359447 - Flags: feedback?(jmuizelaar) → feedback+
Comment on attachment 8359448 [details] [diff] [review]
Part 2.3: gfxImageFormat additional manual changes

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

::: gfx/thebes/gfxD2DSurface.cpp
@@ +41,5 @@
>                               gfxImageFormat imageFormat)
>  {
>      Init(cairo_d2d_surface_create(
>          gfxWindowsPlatform::GetPlatform()->GetD2DDevice(),
> +        (cairo_format_t)(int)imageFormat,

A helper would be nice if you can find a decent place for it.
Attachment #8359448 - Flags: review?(jmuizelaar) → review+
Attachment #8359450 - Flags: review?(jmuizelaar) → review+
Attachment #8359453 - Flags: feedback?(jmuizelaar) → feedback+
Attachment #8359457 - Flags: review?(jmuizelaar) → review+
Attachment #8359458 - Flags: review?(jmuizelaar) → review+
Attachment #8359459 - Flags: feedback?(jmuizelaar) → feedback+
Attachment #8359462 - Flags: review?(jmuizelaar) → review+
Attachment #8359463 - Flags: review?(jmuizelaar) → review+
Attachment #8359464 - Flags: feedback?(jmuizelaar) → feedback+
Funny, we did have one mismatched-enum bug revealed by this change, I didn't see it before because it is B2G specific and B2G tryserver has been busted:

https://hg.mozilla.org/integration/mozilla-inbound/rev/38c297cc1bab
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.