Closed Bug 389729 Opened 13 years ago Closed 13 years ago

Remove non-cairo OS2 gfx code from the tree

Categories

(Core Graveyard :: GFX: OS/2, enhancement)

All
OS/2
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mats, Assigned: mats)

References

Details

Attachments

(2 files)

 
Attached patch Patch rev. 1Splinter Review
In addition to the patch, all files in gfx/src/os2/ will be cvs removed
except the following:
nsGfxDefs.h
nsOS2Uni.cpp
nsOS2Uni.h
nsPaletteOS2.cpp
nsPaletteOS2.h
nsPrintOS2.cpp
nsPrintOS2.h

which are still built for cairo-os2 builds:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/gfx/src/Makefile.in&rev=1.159&root=/cvsroot&mark=114-125#114
(I have no clue whether the code is actually still used though)
Assignee: mozilla → mats.palmgren
Status: NEW → ASSIGNED
Attachment #274048 - Flags: review?(mozilla)
Mats, thanks for your effort. I would prefer to leave the code in the tree for the moment until we have solved some basic problems of cairo-os2 builds like plugins and printing. It's just easier to have something in the tree to look at instead of having to keep another tree around... 

But I should take a look if the Makefile lines you point out are still necessary.
Yes, those files are indeed still used. They could be moved to widget with a little effort. With a few more changed lines nsPaletteOS2.* and then also nsGfxDefs.h can be made obsolete. I guess that palette management will not work with cairo surfaces any more anyway. I am not sure if a cairo-os2 build will even run on a machine with 256 colors where palette management would be needed.

The patch looks fine to me but as I said, let's postpone the actual change a bit.
Note, the non-cairo stuff is the OS/2 code blocks cleanup of gfxImageFrame.
Blocks: 367281
Depends on: 393013
No longer blocks: 367281
Comment on attachment 274048 [details] [diff] [review]
Patch rev. 1

OK, the obsolete code is starting to annoy me, too, while working on other stuff in nsWindow. So let's get this in soonish.

>Index: widget/src/os2/nsWindow.cpp
[...]
> #include "nsGUIEvent.h"
> #include "nsIRenderingContext.h"
>-#ifdef MOZ_CAIRO_GFX
> #include "gfxOS2Surface.h"
> #include "gfxContext.h"
>-#else
>-#include "nsIRenderingContextOS2.h"
>-#endif
[...]
>Index: widget/src/os2/nsWindow.h
[...]
>-
>-#ifdef MOZ_CAIRO_GFX
> #include <gfxOS2Surface.h>
>-#endif

While doing the above changes to nsWindow, can you move both lines
   #include "gfxOS2Surface.h"
   #include "gfxContext.h"
from nsWindow.cpp to nsWindow.h instead of #include <gfxOS2Surface.h>?

About that files listed in comment 0, let's copy them over to widget/src/os2 and add them to CPPSRCS in the Makefile there for the moment. Then deal with cleaning them up in a separate bug. (I think at this points it's difficult to completely remove all of them.)
Attachment #274048 - Flags: review?(mozilla) → review+
Comment on attachment 274048 [details] [diff] [review]
Patch rev. 1

Not sure if approval is required since this is OS/2 only, but asking anyway...
Attachment #274048 - Flags: approval1.9?
Depends on: 394857
Mats, no OS/2 doesn't need approval. To quote mconnor "For anything OS/2 only I think we'd have a blanket approval at this point." (from mozilla.dev.planning).

But before checking this is we should handle the files from comment 1 somehow. I had forgotten about that but now I filed bug 394857 about them.
Blocks: 395491
Attachment #274048 - Flags: approval1.9?
Ok, I addressed your comments and I was just about to land it when
I saw bug 394857 was reopened.  I'll hold off until that bug is resolved.
Attached patch Patch as landedSplinter Review
Checked in to trunk at 2007-09-08 09:22 PDT.

-> FIXED
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.