Add GC-cache to Xprint module

VERIFIED FIXED in mozilla0.9.2

Status

P3
enhancement
VERIFIED FIXED
18 years ago
8 years ago

People

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

Tracking

Trunk
mozilla0.9.2
All
Linux
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Assignee)

Description

18 years ago
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...
(Assignee)

Comment 1

18 years ago
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
(Assignee)

Comment 2

18 years ago
Accepting, setting milestone...
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.2
(Assignee)

Updated

18 years ago
Blocks: 85527
(Assignee)

Comment 3

18 years ago
ToDo:
- Fix "[Bug 86267] Xlib: form checkboxes are not drawn" in Xprint module
         
(Assignee)

Updated

18 years ago
Blocks: 71669
(Assignee)

Updated

18 years ago
Blocks: 72216
(Assignee)

Updated

18 years ago
Blocks: 77060
(Assignee)

Comment 4

18 years ago
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
(Assignee)

Comment 5

18 years ago
Created attachment 38883 [details] [diff] [review]
Patch for 2001-06-16-08-trunk + patch 38719 for bug 80224
(Assignee)

Comment 6

18 years ago
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
(Assignee)

Updated

18 years ago
Blocks: 79228
(Assignee)

Comment 7

18 years ago
Created attachment 38939 [details] [diff] [review]
Better patch for 2001-06-16-08-trunk + patch 38719 for bug 80224
(Assignee)

Comment 8

18 years ago
pocemit:
Wanna r= the patch, please ?
(Assignee)

Comment 9

18 years ago
Created attachment 38990 [details] [diff] [review]
New patch for 2001-06-16-08-trunk + patch 38719 for bug 80224 - fixed issues digged-out by timeless

Comment 10

18 years ago
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.
(Assignee)

Comment 11

18 years ago
Created attachment 39188 [details] [diff] [review]
New patch for 2001-06-18-08-trunk + patch 38719 for bug 80224 to patch r=pocemit/timecop
(Assignee)

Comment 12

18 years ago
s/patch/match/

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

Requesting sr= - tor/blizzard ?
(Assignee)

Comment 14

18 years ago
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.
(Assignee)

Comment 16

18 years ago
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...

Comment 17

18 years ago
dbaron's right, those cases that use ../xlib/ should be using VPATH as we
discussed on irc. 
(Assignee)

Comment 18

18 years ago
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= ...
(Assignee)

Updated

18 years ago
Blocks: 86291
(Assignee)

Comment 19

18 years ago
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.
(Assignee)

Comment 21

18 years ago
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 ?

Comment 22

18 years ago
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.

(Assignee)

Comment 23

18 years ago
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.
(Assignee)

Updated

18 years ago
Blocks: 73434

Comment 25

18 years ago
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.
(Assignee)

Comment 27

18 years ago
Patch has been checked-in by cls (THANKS !!), all staticbuild issues have been
solved in bug 86291.

making bug as FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
(Assignee)

Updated

18 years ago
Blocks: 72028

Comment 28

18 years ago
verifying
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.