Closed Bug 418104 Opened 13 years ago Closed 13 years ago

Remove non-cairo Windows gfx code from the tree

Categories

(Core Graveyard :: GFX: Win32, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mozilla, Assigned: mozilla)

References

Details

Attachments

(2 files, 2 obsolete files)

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.
Attached patch remove it and its reference (obsolete) — Splinter Review
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?
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Attachment #303917 - Flags: superreview?(pavlov)
Attachment #303917 - Flags: review?(pavlov)
I think gfx/imgScaler.{h|cpp} can also be removed with this code, since they are only used by nsImageWin.cpp.
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?
Blocks: 410114
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:40-mozilla@weilbacher.org-macwingfxremoval/
Attachment #303917 - Attachment is obsolete: true
Attachment #306729 - Flags: superreview?(pavlov)
Attachment #306729 - Flags: review?(pavlov)
Attachment #303917 - Flags: superreview?(pavlov)
Attachment #303917 - Flags: review?(pavlov)
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.
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.
Blocks: 418703
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 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.
Attachment #306729 - Flags: review?(pavlov) → review+
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.
Attachment #306729 - Attachment is obsolete: true
Attachment #309397 - Flags: superreview?(pavlov)
Attachment #309397 - Flags: review+
Attachment #306729 - Flags: superreview?(pavlov)
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.
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
Attachment #309481 - Flags: superreview?(pavlov)
Attachment #309481 - Flags: review+
Attachment #309481 - Flags: superreview?(pavlov) → superreview+
Attachment #309397 - Flags: superreview?(pavlov) → superreview+
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.
Attachment #309397 - Flags: approval1.9?
Comment on attachment 309481 [details] [diff] [review]
only changed files

a=beltzner
Attachment #309481 - Flags: approval1.9+
Changes and file removals checked into trunk.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.