Closed Bug 83242 Opened 19 years ago Closed 19 years ago

Add GC-cache to Xprint module

Categories

(Core Graveyard :: Printing: Xprint, enhancement, P3)

All
Linux
enhancement

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.2

People

(Reporter: roland.mainz, Assigned: roland.mainz)

References

Details

Attachments

(4 files)

Xprint does not have a GC-cache yet, that slows down it a bit. May also be
usefull to keep code in sync with Xlib-toolkit...
Swapping owner<-->QA.
Adding dependicy to bug 66082 ("add gc-cache to xlib port")...
Assignee: katakai → Roland.Mainz
Depends on: 66082
QA Contact: Roland.Mainz → katakai
Accepting, setting milestone...
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.2
Blocks: 85527
ToDo:
- Fix "[Bug 86267] Xlib: form checkboxes are not drawn" in Xprint module
         
Blocks: 71669
Blocks: 72216
Blocks: 77060
Adding dependicy to bug 80224 ("Xlib-toolkit's nsFontMetricsXlib.cpp is missing
tons of nsFontCharSetMap mappings...") which fixes tons of i18n issues in
Xlib-toolkit land...
Depends on: 80224
ToDo:
- Add API to nsFontMetricsXlib.cpp to release global vars
- Add assert for access to fonts before XpSetContext() was called (e.g.
accessing fonts without having a valid font context) - this is BAD and should be
avoided !!
- Sync to more changes in bug 80224
- More code cleanup
Blocks: 79228
pocemit:
Wanna r= the patch, please ?
As the majority of the code comes from Xlib, this looks good to me.
Use diff -uw though, so that all these spacing changes dont get included. :)

r=pocemit (if that means anything)

> +INCLUDES       += -I$(srcdir)/../xprint
extra tab, please fix

> +
> +  static PRPackedBool mPrintermode;
I am guessing you decided to go with gPrinterMode and make it static, and
this is not under USE_XPRINT, so I guess it should be removed


> +  mPrintContext =    nsnull;
> +  mContext          = nsnull;
should probably keep indentation consistent


-
+  
lots of these all over the place :) Next person after you is going to 
remove them and make another large patch. Also there is a lot of extra line
ending spaces.
s/patch/match/

Fixed all issues listed in pocemit's review except the space/indentation
stuff...

Requesting sr= - tor/blizzard ?
kin:
Thanks !

requesting a= drivers...
@@ -41,11 +41,12 @@
 
 CPPSRCS
	= \
 
	nsDeviceContextXP.cpp \
-
	nsFontMetricsXP.cpp \
+
	../xlib/nsFontMetricsXlib.cpp \
 
	nsRenderingContextXP.cpp \
 
	nsGfxFactoryXP.cpp \
 
	nsXPrintContext.cpp \
 
	../xlib/nsRegionXlib.cpp \
+
	../xlib/nsGCCache.cpp \
 
	$(NULL)
 


Isn't this going to mess up the build system in certain cases (like storage of
compile-generated dependencies)?  I remember the various hacks needed to allow
building of source files in 2 places for zlib/standalone, libjar/standalone, etc.
dbaron:
AFAIK no, works here with Sun Workshop and gcc builds and GTK+/Xlib-toolkits 
without problems (there may be a small risk for Xlib-toolkit port (but
nsFontmetricsXlib.o is build with the _same_ flags/defines in xlib/ and xprint/
dirs) - but it is not part of the default build and I am monitoring this. And
I'll fix that isse - see below).

And this stuff will be removed soon - shared code gets a new module - this is
bug 85527...
dbaron's right, those cases that use ../xlib/ should be using VPATH as we
discussed on irc. 
cls:
If you could remember your solution... ;-(
I can fix this as part of bug 86291 if you want - please let this bug get it's
a= ...
Blocks: 86291
cls:
Thanks.
Issue will be killed as part of bug 86291...

Requesting a= for this bug, please...
a=dbaron for trunk checkin (on behalf of drivers), conditional on fixing the
build system changes to cls's approval.  I really don't want to see something go
in that breaks builds (did you try srcdir and objdir builds?), and it shouldn't
be hard for you to fix this.
dbaron:
Agreed. bug 86291 will be fixed tomorrow - cls needs it urgendly for his
StaticBuild stuff. This issue should be fixed before friday (<= 2 days...)...

Thanks !

----

mkaply@us.ibm.com - wanna do the checkin, please ?
Provided that the current patch builds in an objdir and I have Roland's
guarantee that the VPATH issue will be fixed in tomorrow's carpool, I have no
problem with the patch going in now.  If it doesn't build in an objdir most of
the tinderboxes will break so it should be tested before it is checked in.

cls wrote:
> I have Roland's guarantee that the VPATH issue will be fixed in tomorrow's 
> carpool

I've added this issue to the ToDo list in bug 86291...
And I always build my Mozilla (WS6U2, gcc both SPARC and x86) with:
% mkdir objdir; cd objdir
% ../configure myoptions

No problems here...
I would prefer not to take ownership for checking this in if it has the 
possibility of breaking builds.

I can't test build it.

All the other patches I have checked in have been code that wasn't built by 
default.
Blocks: 73434
So anyway, after having a fruitless discussion with Roland on irc for the past
1/2 hr, does he or does he not have approval to check this patch in?  The
objectionable part that would need to be fixed with VPATH was partially fixed in
the static build landing this afternoon.
cls:  This has approval if you're happy with the build system stuff.
Patch has been checked-in by cls (THANKS !!), all staticbuild issues have been
solved in bug 86291.

making bug as FIXED.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Blocks: 72028
verifying
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.