418104 - Remove non-cairo Windows gfx code from the tree

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Win32, defect)

Description

[Peter Weilbacher]

The whole tree in gfx/src/windows is still there and should be removed, together with references to it.

For the moment I only found that toolkit/toolkit-makefiles.sh refers to it, but there might be others that just don't spell it out. intl/unicharutil/util/nsBidiUtils.cpp and layout/base/nsBidiPresUtils.cpp have comments that refer to this directory but I don't think those should be touched (after all, the comments are still true).

As the old windows gfx files still refer to FONT_LEADING_APIS_V2 this blocks bug 415686.

Comment 1

[Peter Weilbacher]

Attachment: remove it and its reference

Probably another can that I should better not open but here it goes. (Only the first hunk is a real patch the rest is just the contents of the files to be removed.)

I found a reference to nsFontMetricsWin.cpp in gfx/src/shared/ignorable.x-ccmap but as that is a generated file I better not try to edit it. Anything else that I might have forgotten?

Comment 2

[Jeremy Lea]

I think gfx/imgScaler.{h|cpp} can also be removed with this code, since they are only used by nsImageWin.cpp.

Comment 3

[Peter Weilbacher]

Thanks Jeremy, could catch. :-)

In the meantime I also remembered to search for gkgfxwin to see where the library is still referred to, and found these files:

/minimo/base/wince/minimo-link-comps,
    * line 10 -- gkgfxwin
/minimo/base/winnt/minimo-link-comps,
    * line 10 -- gkgfxwin
/tools/preloader/preloader.cpp,
    * line 215 -- "gkgfxwin;gkwidget;" \
    * line 287 -- "gkgfxwin;gkwidget;" \
/embedding/config/basebrowser-installer-win.pkg,
    * line 64 -- components\gkgfxwin.dll
/embedding/config/basebrowser-win,
    * line 73 -- components\gkgfxwin.dll 

Are minimo and the embedding configuration changed to cairo (who can tell me)? If so I think that should be removed. And is that preloader still used?

Comment 4

[Peter Weilbacher]

Attachment: remove gfx/src/windows and its references (v2)

OK, I convinced myself that the minimo and preloader files are outdated anyway (they still don't contain gkgfxthebes), so I don't think we need to patch them in this bug. But I added the embedding/config/basebrowser*win* files to the patch. Everything else is unchanged from the previous patch.

I also ran this through the try server and while the Linux and Mac boxes had problems with the patch, the Windows machine ran through just fine and put builds up here:
https://build.mozilla.org/tryserver-builds/2008-03-01_02:[e-mail]-macwingfxremoval/

Comment 5

[Peter Weilbacher]

Stuart, if you don't have time to review this can you point out another reviewer, please? This blocks a wanted-1.9 bug, so I would like to get it in before 1.9b5.

Comment 6

[Alfred Kayser]

Before this can be reviewed (I could do the 'r' part), you need to address the problems that the Linux and Mac boxes had in your trybuild. The removal itself is for Windows safe enough, but it needs to be sure that all dependencies that other platforms might have to do this are cleared.

I would also love to have this old code removed.

Comment 7

[Peter Weilbacher]

The problems with Mac and Linux on the try server were entirely at the patching level and will _not_ cause problems when the patch gets checked into CVS. It was about some weird line-break in one of the Mac files that patch cannot handle correctly on all three platforms simultaneously. See the logs
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1204368032.1204368167.16724.gz
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1204368032.1204368167.16724.gz

Comment 8

[Alfred Kayser]

Comment on attachment 306729 [details] [diff] [review]
remove gfx/src/windows and its references (v2)

Bug 418703 already tried to patch this old code, so let's first remove the old code before doing any great Windows changes...

And don't forget to also remove imgScaler.* (or create a separate bug for it)

With that, r=me, so that Stuart only has to do the 'sr', knowing that at least someone else has reviewed it.

Comment 9

[Peter Weilbacher]

Attachment: remove gfx/src/windows, its references, and imgScaler (v3)

Thanks for the review and the hint about imgScaler. By now I verified that it is unused outside the Windows code, and so I updated the patch to include it.

Comment 10

[Stuart Parmenter]

Comment on attachment 309397 [details] [diff] [review]
remove gfx/src/windows, its references, and imgScaler (v3)

if you could just post a list of removed files and then a diff of actual changes that would be nice.

Comment 11

[Peter Weilbacher]

Attachment: only changed files

Sure, that is easily done.

Three trivially changed files, patch in this attachment. The files to be removed are:

gfx/src/windows/.cvsignore
gfx/src/windows/Makefile.in
gfx/src/windows/blank_glyph.ccmap
gfx/src/windows/fontEncoding.properties
gfx/src/windows/fontNameMap.properties
gfx/src/windows/gfxwin.pkg
gfx/src/windows/nsDeviceContextWin.cpp
gfx/src/windows/nsDeviceContextWin.h
gfx/src/windows/nsDrawingSurfaceWin.cpp
gfx/src/windows/nsDrawingSurfaceWin.h
gfx/src/windows/nsFontMetricsWin.cpp
gfx/src/windows/nsFontMetricsWin.h
gfx/src/windows/nsGfxFactoryWin.cpp
gfx/src/windows/nsIDrawingSurfaceWin.h
gfx/src/windows/nsIRenderingContextWin.h
gfx/src/windows/nsImageWin.cpp
gfx/src/windows/nsImageWin.h
gfx/src/windows/nsRegionWin.cpp
gfx/src/windows/nsRegionWin.h
gfx/src/windows/nsRenderingContextWin.cpp
gfx/src/windows/nsRenderingContextWin.h
gfx/src/windows/nsUnicodeRange.cpp
gfx/src/windows/nsUnicodeRange.h
gfx/src/imgScaler.cpp
gfx/src/imgScaler.h

Comment 12

[Peter Weilbacher]

Comment on attachment 309397 [details] [diff] [review]
remove gfx/src/windows, its references, and imgScaler (v3)

I guess this should wait until after the beta 5 freeze.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 309481 [details] [diff] [review]
only changed files

a=beltzner

Comment 14

[Peter Weilbacher]

Changes and file removals checked into trunk.