57820 - Xprint does not scale images to suit the higher printer resolution

Status: RESOLVED FIXED

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

Description

[ekrock's old account (dead)]

[ This is a replacement bug report for bug 54095.]

Images seem to be copied directly to the higher resolution (300dpi) drawable
pixel-for-pixel without scaling.  For example, printing a web page designed for
roughly an 800 pixel width, will print (at 300 dpi) at less than 3" across -
literally the same 800 pixels.  I can fudge the text size with mTextZoom in
xprintcontext to 2.0f or 3.0f but this doesn't help for images of course.

All those calculations in nsDeviceContextXP::InitDeviceContextXP to establish
mAppUnitsToDevUnits look like they should add up to produce a scaling factor
between the screen res and printer res, but even hardcoding in values here
doesn't affect the image size.  Perhaps I'm misunderstanding the purpose of
these calculations.

Is this a bug, a 'feature' or am I just interpreting the data incorrectly?


------- Additional Comments From Brad Quinn 2000-09-26 07:33 -------

*** Bug 54169 has been marked as a duplicate of this bug. ***


------- Additional Comments From [e-mail] 2000-09-28 14:25 -------

This is an xprint issue since mozilla/windows scales up the images
appropriately, as requested with 54092 I am reassigning this bug to
[e-mail].


------- Additional Comments From [e-mail] 2000-10-10 14:33 -------

This is needed. Hidetoshi, can you estimate the work involved?


------- Additional Comments From Frank Tang 2000-10-10 16:37 -------

dougt , why this is rtm ?


------- Additional Comments From [e-mail] 2000-10-11 10:42 -------

ftang, IIRC this is a blocker for embedding work according to ThomasVL per
recent conference call. ThomasVL, please provide whatever additional details you
can about why this matters within a public bug report. In the meantime, ftang,
this must be fixed in the RTM timeframe.

petitta: I think this is one of the four bugs that were discussed on Friday,
yes?


------- Additional Comments From TVL 2000-10-11 14:24 -------

we got a producted based to gecko that includes printing on some non PS devices,
to do this, it uses XPrint.  Since the images aren't scale, they end up tiny
compared to the rest of the html document.  (yes, the print code will hopefully
end up as OSS in the end).


------- Additional Comments From Hidetoshi Tajima 2000-10-16 20:18 -------

I don't think I can fix this in time for rtm time frame. Reassign to Frank
Petitta for now
as he's willing to look for another engineer.


------- Additional Comments From [e-mail] 2000-10-18 17:51 -------

Will the engineer assigned to this bug please contact [e-mail] ASAP


------- Additional Comments From [e-mail] 2000-10-19 08:27 -------

I don't have anything to do with xprint.  I don't even have a linux (unix) box
to work on.  Giving back to petitta.


------- Additional Comments From [e-mail] 2000-10-24 09:50 -------

Some questions:

How to configure XFree86 to support the Xprint extension?
Is it possible to set up an Xprint daemon on Linux? We assume that this is the
daemon we need to support for embedded environments, but we have found no info
on the net regarding Xprint on Linux.

Comment 1

[Kevin McCluskey (gone)]

Reassigning to Waqar

Comment 2

[Hidetoshi Tajima]

XPrint installation on XFree86 is broken. At least you need to
install Xprint configuration files in /usr/X11R6/lib/X11/C/print/
directory. They are included in lateset XWindow System, X11R6.5.1,
as xc/programs/Xserver/XpConfig/. See README file for more about
installation.

Comment 3

[TVL]

the alternate server that comes with Xprint doesn't seem to be needed.  We're 

working on the last legal issues, but should be able to push all the code related 

to this printing into the embedding branch shortly.

Comment 4

[waqar]

Tajima-san I have got config files for Xprint setup on my linux box. How do I 
test mozilla printing using Xprint rather than the standard lpr printing? 
Thanks.

Comment 5

[Hidetoshi Tajima]

Xprint modules does not build by default, so you may need to
rebuild mozilla first. run "configure --with-xprint", and
to build necessary modules only, run "cd mozilla/gfx; make"
Then, in order to use Xprint instead of PostScript, add the
line:
 user_pref("print.print_method", 1);
into your prefs.js and restart mozilla.

Comment 6

[waqar]

Ok, the print out same as with postscript. Either it is not setup correctly or I 
dont see the bug. I am using HP LaserJet4MP postscript printer to test the out 
put.

Comment 7

[waqar]

I talked to [e-mail] he is also working with xprint problem with
HP printers. He is using HP DeskJet 932c USB printer on a linux box to print and
see the problem.

He sent me a build with changes to print using xprint, but I cant reproduce
this  bug because every time I print to my printer it comes out as Postscript
and prints correctly.

Does anybody else have this printer?

Comment 8

[waqar]

I finally got the printer setup. Here is what I did,
Install Redhat 7.0 (kernal 2.2.16) with usb support. (did not work)
Upgrade the kernal to 2.4.0-text7 with usb enabled and usb printer support.
setup up the printer in /etc/printcap

##AUTOLPD##
lp0|My DeskJet 932c:\
        :sd=/var/spool/lpd/lp0:\
        :lp=/dev/usb/lp0:

Make sure that you have write permissions to /dev/usb/lp0 
crw-rw-rw-    1 root     lp       180,   0 Aug 24 04:00 lp0

Now I can print but I get crash in nsXPrintContext.cpp, line 284, by looking
at the contents of x_image the member data is null.

I get printout but it is just junk, can't print mozilla.org home page.

Comment 9

[waqar]

Here is my update, What I have done so far. Gotten a printer from AOL
HP 840c (which is different from [e-mail]), he is using
HP932c.

I have compiled new kernals Linux 2.4.0-test7 and 2.4.0-test10. I am
finally able to print something.

When I print Mozilla.org home page I get two very small images and thats
it. The browser crashes with segfault.

When I try to print Yahoo page I get half of the image with logo and
three buttons on each side of the logo, and locks up the browser.

HP is hard coding the printer to be DJ9xx, I changed that to DJ8xx but
that had no effect on the printing.

Comment 10

[waqar]

Here is the output I get, under the debugger I found that we are trying to
access the data which is null string.

Printing to XPRINT Device...
Init RAS_PRINTER...
SetupRasterPrintJob Called
PrinterIsAlive()
DevID = TRUE
Status = TRUE
pPC constructed
LETTER size paper
before setupwindow
status = 0
Enabling Quirk StyleSheet
Xlib:  extension "XpExtension" missing on display ":0.0".
page size = 5760, 7200
GetBandHeight = 75 DevUnits, 180 AppUnits
bandheight=180, pageheight=7200, numbands=40
Band[0], mOffsetY=0
./run-mozilla.sh: line 72:  1515 Segmentation fault      $prog ${1+"$@"}

Comment 11

[tlogue]

Actually, the printer is not hardcoded.  The printer's devID is read and 
imaging is established based on that model.  The SelectDevice function should 
never be reached because it only comes into play with uni-directional 
communication.  Since USB is inherently bi-directional, if USB is working then 
we can get the devID.
As for the seg fault, hmm.  Try rebuilding with the debug flags -DDEBUG & -
DDBG_MASK=0x02.  You'll find these commented out in the makefile.  This will 
produce HP driver output in addition to the xprint output already shown and 
that way we can see if it's even getting to the driver.
You mention 'the data' is null - can you elaborate on this?  Do you know the 
element that is actually null or do you just know that a null variable has been 
referenced?
Finally, considering the debug code I see, try uncommenting the printf's in 
nsXPrintContext.cpp at the start of ::StartBand and ::EndBand which may narrow 
down the location of the problem.

Comment 12

[waqar]

Here is the output with debug setting.
Printing to XPRINT Device...
Init RAS_PRINTER...
SetupRasterPrintJob Called
DeviceRegistry created
PrinterIsAlive()
DevID = TRUE
Status = TRUE
DR::SelectDevice: model= 'DESKJET 840C'
DR::SelectDevice: pens= '$HB0$FC0'
DR::InstantiateGeneral: device= DJ895 created
pPC constructed
LETTER size paper
before setupwindow
status = 0
Partial Send but not slow poll mode
entering slow poll mode
In Header constructor
Setting MediaType to Plain
Setting MediaSource to Tray1
Setting Quality to Normal
Setting SimpleColor mode
Compression=Mode2
Enabling Quirk StyleSheet
page size = 5760, 7200
GetBandHeight = 75 DevUnits, 180 AppUnits
bandheight=180, pageheight=7200, numbands=40
Band[0], nsXPrintContext::StartBand()
mOffsetY=0
nsXprintContext::Endband
Partial Send but not slow poll mode
entering slow poll mode
still in slow poll mode, count = 2
still in slow poll mode, count = 3
still in slow poll mode, count = 4
Printer slow poll times exceeded
still in slow poll mode, count = 2
Parsing possible error state...
GetStatusInfo() status = 24
DeskJet895: parsing error info
DJ895::ParseError, calling VerifyPenInfo
PrinterIsAlive()
iTotal_SLOW_POLL_Count = 3
< this repeats for few times >
entering slow poll mode
NULL NULL Partial Send but not slow poll mode
entering slow poll mode

The data member I was refering to ::EndPage (now EndBand) the line

RGBtriplet[k]   = (BYTE)(x_image->data[j+2]);
value of j = 14401528 and data == ""

Comment 13

[tlogue]

Wow, this is odd.  Your value of j is way big, so if we're dereferencing data
[j+2], then I'm not surprised we're getting a problem.  Though if we're just 
reading this data I'd think we're more likely to just get garbage rasters.  The 
components from which j is calculated all seem to be init'd correctly.  
mBandHeight is 75 dev units, mWidth is 2400 (which I can see from the page size 
= 5760, 7200 line as there's a 2.4 expansion from dev to app units).  This 
means 0 <= j > 720000.
However, if data[] is actually null, then that j value could have just been 
molested by memory corruption I guess.  This would indicate to me a problem 
with XGetImage, since it's the function responsible for filling out the XImage 
structure.
If you can set a break after XGetImage has been called, you should see that 
x_image has been populated.
Here's what I get for my x_image structure values(names are abbreviated):
x_image->w=2400,h=75,xoff=0,f=2,data=(some ptr. value != 
NULL),bo=0,bu=32,bbo=0,bp=32,d=24,bpl=9600,bpp=32
If this struct did not get filled in correctly, then it would seem we have a 
problem with xlib - most likely in its usage previously rather than an actual 
BUG in xlib.

Comment 14

[waqar]

The crash I am getting is happening due to 16bit and 8bit color. Crash does not
happen on 24bit. According to [e-mail]

24bit Outout if correct.
16bit crash + 2 columns with the same image
8bit  crash + 4 columns with the same image 

Comment 15

[waqar]

Sorry it should read. 24bit Output is correct.

Comment 16

[waqar]

Crash is fixed, now I can print mozilla page without crashing.

nsXPrintContext.cpp
for (j=i*mWidth*iFactor, k=0; j<(i+1)*mWidth*iFactor; j+=iFactor, k+=3)

iFactor needs to be 4 for 24bit, 2 for 16bits and 1 for 8bits.

Comment 17

[waqar]

Here is the response I got from Sun engineer who is working on Xprt.
--Begin Message--
 The problem boils down to this:
 
 Mozilla performs a page layout using images and character metrics.
 This data is correct and consistent when layed out for the screen.
 The images are at 100 DPI and the characters are at 100 DPI. When
 printing to the printer, Mozilla uses 100 DPI images, but 300/600
 DPI characters. These characters are returned by Xprt which knows
 about the printers resolution. Since the character and images do
 not match in DPI, they are layed out incorrectly by Mozilla. The
 information pertaining to the attachments between the various
 pieces of the layout is not sent to Xprt, so the page cannot be
 re-layed out at that level.
 
 Only two solutions exist to this problem. The first is have Xprt
 send down 100 DPI fonts to Mozilla. Mozilla can then perform the
 page layout correctly. The resulting page will be sent back to
 xprt which will perform a scale factor on all of the results to
 change them into the printers resolution. This method is complicated
 by the fact that two seperate fonts will need to be opened for
 each font to be used. One font will be at 100 DPI, to send back
 to Mozilla, and the other at 300/600 DPI, for use by the printer.
 This method could be valuable for other applications like Mozilla
 that want to use xprt, but that do not know how to layout for
 anything other than the screen.
 
 The second solution to the problem would be to make Mozilla know
 about the resolution of the device that it is rendering to. In this
 way, rendering to the screen, Mozilla would know to use 100 DPI.
 When using the printer, Mozilla would scale everything to the printer
 resolution before performing the layout. (Performing the actual
 scaling is unnecessary as the printer can do that, and sending
 large bitmaps over the wire is not optimal. But knowing what the
 resulting image size would be and its position is required). This
 second option would be valuable in making Mozilla more like other
 applications that are capable of this. I.E. Star Office. This option
 may be faster than the first.
 Currently, I am persuing the first option of modifing xprt to handle
 the scaling automatically on all 100 DPI layed out pages provided by
 Mozilla.
-- End Message--

Comment 18

[petitta]

Gents, Where do we stand on this.  Could I please get an updated status ASAP?

Comment 19

[tlogue]

A couple of thoughts on this.  First, I'm not comfortable with the text's PQ 
degradation that would be inherent with Xprt supplying 100dpi rendered text 
which is then scaled up.  For instance you'll find that, say 12pt text, 
rendered at 150 dpi then scaled up is a little jaggy & blurry and ultimately 
straining on the eye.  Less than 150 dpi is usually quite bad.  High dpi on 
text is far more important than on images because text is usually black on 
white while the eye blends colors together in an image.
Secondly, the mention that the printer does the scaling is not really right.  
Yes, you can tell the printer to go into 150 dpi mode and it will dumb-scale 
everything up, but it won't look very good.  We're talking about the consumer 
space where most everything is inkjet - not a lot of memory here compared to 
the big ol' laser printer attached to the big ol' mainframe in a university 
basement.
It seems we could stick a simple pixel-replication scaler inline with the 
xprint renderer's DrawImage function before it gets put into the 300 dpi 
pixmap - but that still doesn't solve the layout issue.  Hmm... This shouldn't 
be THAT difficult...

Comment 20

[shrirang khanzode]

spam : changing qa to sujay (new qa contact for Printing)

Comment 21

[Roland Mainz]

Q: Why are bitmap fonts added to the xprint job ? 
Wouldn't it be usefull to restrict the available fonts in Xprt to the printer's
build-in fonts except for those charsets where the printer does not have
builds-ins for (japanese chars for example) to avoid that the print jobs size
becomes too large (see also bug 68779) ?

----

Changing "component" from "printing" to "printing: Xprint" (we now have our own
bug component... :-)

Comment 22

[Kevin McCluskey (gone)]

Setting target milestone to future

Comment 23

[Roland Mainz]

Font quality issues described here may simply be fixed by removing bitmap fonts
from font path, e.g. only using "scaleable" fonts.
It would be better to use "printer internal fonts" first - otherwise the print
job's size quickly reaches unuseable values - but this would require a minor
modification of current Xprt implementation and HP folks to release their
printer "models"(=config files in $XPCONFIGDIR/C/print/models/) (current Xprint
in X11R6.5.1 only comes with three sample configs - but AFAIK HP hoards much
more of these nice files... ;-( ).

----

OT: Anyone here interested in a "X11 print system" mailinglist ?

Comment 24

[Kevin McCluskey (gone)]

Reassigning to xprint owner

Comment 25

[Masaki Katakai]

Assign to Roland

Comment 26

[Roland Mainz]

Accepting bug.

Comment 27

[Roland Mainz]

Assumung bug 77210 catches the 0.9.1 train this bug should the "fixable" before
0.9.2 lands...

Thoughts/ToDo (braindump to disk):
- nsXPrintContext::DrawImageBits() doesn't work with scaling/alpha etc. ...
looks like XPutImage(destPixmap); XCopyPixmap(destPixmap,window); doesn't scale
- but XPutImage(window) does (this looks correct for me... no Xprt-bug... :-)...
- xlibrgb.c has some insane ideas about cutting images into smaller tiles. May
be good for performance but screws-up tons of things (dxcp/lbx/Xprint image
scaling/etc.). BAD. I file a hack as workaround for now...
- OT: nsRenderingContextXP starts with mCurrentColor==white... paper is white...
good thought... but X11 background is always _black_. Fix it...
Short workaround for DrawImageBits()+non-alpha images:
-- snip --
// Draw the bitmap, this draw just has destination coordinates
nsresult
nsXPrintContext::DrawImageBits(PRUint8 *alphaBits, PRInt32  alphaRowBytes,
                               PRUint8 *image_bits, PRInt32  row_bytes,
                               PRInt32 aX, PRInt32 aY,
                               PRInt32 aWidth, PRInt32 aHeight)
{ 
  PR_LOG(nsXPrintContextLM, PR_LOG_DEBUG,
("nsXPrintContext::DrawImageBits(%d/%d/%d/%d)\n",
         (int)aX, (int)aY, (int)aWidth, (int)aHeight));


  if( alphaBits == nsnull )
  {
    GC         gc;
    XGCValues  gcv;
    
    // Write into the pixemap that is underneath gdk's alpha_pixmap
    // the image we just created.
    memset(&gcv, 0, sizeof(XGCValues));
    gcv.function = GXcopy;
    gc = XCreateGC(mPDisplay, mDrawable, GCFunction, &gcv);
    
    xlib_draw_rgb_image(mDrawable,
                        gc,
                        aX, aY, aWidth, aHeight,
                        XLIB_RGB_DITHER_XPRINT,
                        image_bits, row_bytes);
    
    XFreeGC(mPDisplay, gc);
    return NS_OK;
  }                      

  Pixmap   alpha_pixmap  = None;
  Pixmap   image_pixmap  = None;
-- snip --

Comment 28

[Roland Mainz]

Attachment: Workaround for xlibrgb.c

Comment 29

[Roland Mainz]

Attachment: http://de.movies.yahoo.com/ printed via Xprint

Comment 30

[Roland Mainz]

I've attached a PS job from S7 Xprt printed with Mozilla5/Xprint module...

Comment 31

[Roland Mainz]

Attachment: Patch for 2001-05-28-08-trunk(+patch from bug 77210)

Comment 32

[Roland Mainz]

Can anyone review the patch, please ? blizzard ?

Comment 33

[Roland Mainz]

Per blizzard's comment in
news://news.mozilla.org/netscape.public.mozilla.reviewers
> I'm punting to tor on this one since he knows drawing code better than I do.

tor, wanna give me r=tor, please ?

Comment 34

[tor]

I thought the idea of this patch was to get rid of GetScaledXImage and
company, which you promised were temporary in a prior xprint bug.  They
still seem to be in use.

Bitmap and pixmap allocation in xlibrgb - you're not taking into account
the scanline pad.  Allocate height*image->bytes_per_line;

Comment 35

[Roland Mainz]

> I thought the idea of this patch was 
> to get rid of GetScaledXImage and
> company, which you promised were
> temporary in a prior xprint bug.  
> They still seem to be in use.

No and yes.
The code path which uses GetScaledXImage() is only used if something goes wrong
(XpSetImageResolution() fails, factorX!=factorY).

The common code path is to use the solution via XpSetImageResolution(), e.g.
XpSetImageResolution()+"using DrawImage() [shortcut]" - which completely
bypasses the client-side scaling code(=GetScaledXImage()&&friends)...

GetScaledXImage() is only in the source to catch very very rare cases.

I agree with you that GetScaledXImage() is not a good nor a fast solution. It is
only a dumb but short&easy solution to gurantee that the code _always_ works
_correct_ (just to avoid that anyone opens a bug and adds the keyword
"correctness"... :-)...

The entire code may be replaced by "stealing" some code from nsImageXlib - but
that scaling code (better/faster) didn't land yet...
(the next big think is to pull-over the GC-cache from Xlib-toolkit to get rid of
some ugly font issues - and then I have time to crawl through the code and think
about smarter solutions for some things...).

> Bitmap and pixmap allocation in 
> xlibrgb - you're not taking into 
> account the scanline pad.  Allocate 
> height*image->bytes_per_line;

Ouch. Fixed.

Going to file new patch to get r= ...

Comment 36

[Roland Mainz]

Attachment: New patch for 2001-06-04-08-trunk

Comment 37

[Roland Mainz]

Filed new patch.

Changes:
- Fixed bad intendation in xlibrgb.[ch] and some TABs (thanks to timecop/pocemit
for that hint :-)
- Fixed bitmap/pixmap malloc() in xlibrgb.c
- Fixed xlibrgb.c to use the given depth in xlib_rgb_init_with_depth() - and
fall back to ignoring that depth if there is no matching visual... - partial fix
for bug 80562
- Fixed colormap issue at XCreateWindow() time in nsXPContext (partial fix for
bug 80562)
- Added ENV var "MOZILLA_XPRINT_EXPERIMENTAL_USE_24BIT_VISUAL" (this env var is
only a _temporary_ thing for external testers) to give other people an easy way
to dig-out why Xprt PS DDX with 24bit visual prints images correct but all text
looks reverse-b/w (see bug 80562)... (I assume something in xlibgb.c or my code
is wrong...)

Comment 38

[Roland Mainz]

tor:
Wanna r= the new patch, please ?

Comment 39

[timeless]

ok r=timeless for the patch w/ minor comment fixes.

Comment 40

[Roland Mainz]

Attachment: New patch for 2001-06-04-08-trunk to match r=timeless

Comment 41

[Roland Mainz]

Filed new patch to match r=timeless
Changes:
- changes in the comments
- commented-out tiling stuff in xlibrgb.c - the code is still available in the
comment as an example for _bad_ ideas

Comment 42

[Roland Mainz]

tor/blizzard:
Wanna give me sr=, please ?

Comment 43

[tor]

Have you discussed your changes with the xlib port people so that they
know what you're considering for xlibrgb and have a chance to review it?

Comment 44

[Roland Mainz]

tor:
Good question. timeless reviewed it, blizzard is in the CC: of this bug and
timecop(=pocemit) is aware of this bug, too (see [e-mail], he's in
the CC: of the first r= request).

But: the patch does not affect the functionality of the code - I don't see
problems here...

Comment 45

[tor]

I think the xlib port people would like to have the tiling code to prevent
the memory spike needed to allocate the whole XImage for a large image.

CCing xlib people for their perspective.

Comment 46

[Roland Mainz]

tor:
The new patch has a comment about that.
a) the XFlush() is far more worse than the malloc(). malloc() costs some CPU and
no context-switches, the XFlush() usually results in two or more
context-switches.
b) The malloc() can be replaced by allocating a static buffer (no tile buffer!!)
for the XImage data, the XImage itself may be allocated from stack. I can file a
new patch on demand...

BTW: Should code in xlibrgb.c be thread-safe (old and new code isn't thread-safe
yet...) ?

Comment 47

[Roland Mainz]

Attachment: New patch for 2001-06-04-08-trunk

Comment 48

[Roland Mainz]

Filed new patch to make "tor" happy.

Changes:
- Replaced XCreateImage() and malloc() by XImage allocated from stack and a
static buffer which gets allocated once. malloc() will still be used if the
image data do not fit into that buffer - but this is a very rare case (for
example: full-screen images in 24bit - but in those cases the malloc()-overhead
is a ridiculous small amount of CPU time compared to other stuff (like the
conv()-call)...).
- Better error checking. Anything goes wrong xlib_draw_rgb_image_core() do not
poke NULLptrs anymore...

Could anyone please r=/sr= that new patch, please ?

Comment 49

[Roland Mainz]

Sorry...

s/Anything goes wrong/If anything goes wrong/

Comment 50

[tor]

I'm pretty sure that only xshm images are async copying the image data, so
you're probably safe with the simpler change of just knocking out the XFlush().

I don't think you understood my comment about memory use.  With your changes,
if I load a large image from the cache (example: 3040x2064 nasa mission image)
xlibrgb will temporary grab a 24mb chunk of memory for the ximage.  Before it
would have just shuffled it through its set of tiles.

Comment 51

[Roland Mainz]

tor:
Yes... I know. But using the "tiles" breaks some things. For example - the image
cache hit rate in DXCP and LBX drops until it becomes more or less useless
because it is spammed with much more images (and it may happen that the same
image passed to xlib_draw_rgb_image() runns as different tiles over the wire
(--> more cache spam)). More images means more work for the CPU/OS (context
switches). I really don't know what is better: Using malloc() or using the
tiles. The tiles just avoid the use of malloc(). But is this really good in all
cases ?
Hint: malloc() memory is just some logical space from virtual memory. Only if
the application touches it it will result in real page allocations - and unused
pages should be moved away...
On demand (if you like it) I can replace the malloc() of the static buffer with
a mmap(anonymous_memory) of 64MB (which means the the available adress space of
mozilla-bin will shrink - not the amount of available virtual memory)... =:-)

And last but not least: This tiling-stuff is totally incompatible to scaling via
XpSetImageResolution() (which keeps the X/Y coordinates and only adjusts
width/height of the resulting image in PS/PDF/PCL job)... ;-(

Comment 52

[Roland Mainz]

tor:
We can test that. Please implement this fix now that Xprint works - and then we
can do further (this doesn't work now - images in Xlib-toolkit are still
busted...) testing using things like the ZDnet browser benchmark and check which
solution is better...

Comment 53

[Roland Mainz]

Attachment: New patch for 2001-06-04-08-trunk incl. xlib_disallow_image_tiling()

Comment 54

[Roland Mainz]

To end the discussion and speed-up this bug a little bit:
I have reestablished the old code and added a new function
"xlib_disallow_image_tiling()" which controls whether the code uses image tiling
or not.

Requesting sr= for that patch, please...

Comment 55

[tor]

You might want to get xprint compiling before asking for review and
super-review:

/home/tor/src/mozilla/gfx/src/xprint/nsFontMetricsXP.cpp: At top level:
/home/tor/src/mozilla/gfx/src/xprint/nsFontMetricsXP.cpp:3413: syntax error
before `::'

Comment 56

[tor]

You're not turning tiling back on after xprint runs.

Comment 57

[timeless]

ok I fixed the bustage introduced by 1.18 <[e-mail]> 08 Jun 2001 
16:36 for bug 69117 -- which wasn't synced to changes by Roland per tor.

s/XP/Xp/ silly case stuff.

Comment 58

[Roland Mainz]

> You're not turning tiling back on after xprint runs.

This is not neccesary. Xprint-module owns it's "own copy" if the xlibrgb module.
Even if Xlib-toolkit is used - both modules share the same source - but
(unfortunately; bloat... there should be a better way...) not the same
object/library code (er... this wouldn't work anyway due the global
variables...)...

Comment 59

[Roland Mainz]

Attachment: New patch for 2001-06-10-08-trunk

Comment 60

[Roland Mainz]

Filed a new patch to get rid of an issue when printing pages like
http://www.autosupply.co.nz/ 
I am now taking the image scaling value from devicecontext's cannonical pixel
scaler value - which is mainly identical to my self-calculated value... except
some rare cases like this one... ;-(

Requesting r= and sr= ...

Comment 61

[timeless]

r=timeless w/ a few more nits

Comment 62

[Roland Mainz]

Attachment: New patch for 2001-06-10-08-trunk to match r=timeless

Comment 63

[Roland Mainz]

Filed new patch to fixed all issues shown-up in timeless's review...

tor:
Requesting sr=, please...

Comment 64

[tor]

xlibrgb:

 * use NS_WARNING or NS_ERROR instead of NS_ASSERTION(PR_FALSE, ...)

 * move size for static_buffer_size into a #define with a reasonable name

xprint:

 * rename MY_XLIB_RGB_DITHER_XPRINT to NS_XPRINT_RGB_DITHER

 * add call to xlib_disallow_image_tiling() to turn back on tiling for 
     when xlibrgb code is shared

general:

 * file a bug about turning xlibrgb into a shared library that both xlib
     and xprint can link to

Comment 65

[Roland Mainz]

tor:
Sharing xlibrgb code is currently _impossible_ as the whole code and all sources
which use it heavily rely on static variables which hold |Display *|, |Screen *|
etc. The whole toolkit (including GTK+/Xlib/Qt/etc.-toolkits and the base
classes like nsIDeviceContext !!) system needs to be rewritten to get that
working properly.
I really wish that this would be possible... but it is far too much work -
that's why there is a gfx2/ tree (assumption... :-) ...

Comment 66

[Roland Mainz]

Filed bug 85527 per tor's request.
Well... it is not impossible... but it will move a _lot_ of code... and the size
of the resulting patch will make it rotten very very fast... which makes coding
a lot of fun... ;-(

Comment 67

[Roland Mainz]

Attachment: New patch for 2001-06-10-08-trunk to make tor happy... :-)

Comment 68

[Roland Mainz]

> xlibrgb:
>  * use NS_WARNING or NS_ERROR instead of NS_ASSERTION(PR_FALSE, ...)

Fixed.

>  * move size for static_buffer_size into a #define with a reasonable name

Fixed. Search for XLIB_STATIC_IMAGE_BUFFER_SIZE ... :-)

> xprint:
>  * rename MY_XLIB_RGB_DITHER_XPRINT to NS_XPRINT_RGB_DITHER

Fixed.

>  * add call to xlib_disallow_image_tiling() to turn back on tiling for 
>      when xlibrgb code is shared

xlib_rgb_detach() is doing the job - just to avoid that this must be "done"
explicitly. But bug 85527 will introduce a smarter solution for this... :-)
Fixed.

> general:
>  * file a bug about turning xlibrgb into a shared library that both xlib
>     and xprint can link to

Done. This is now know as bug 85527...

Wanna give me sr= for that patch, please ?

Comment 69

[tor]

sr=tor

Comment 70

[Roland Mainz]

asa:
Requesting a=asa/[e-mail] to get the patch into "trunk"...

Comment 71

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 72

[Roland Mainz]

asa:
Thanks !

CC:'ing [e-mail] for checkin:
mkaply... wanna do the checkin and mark the bug as "fixed" after checkin ?
Thanks !

Comment 73

[Mike Kaply [:mkaply]]

Checked in - marking fixed.

Checking in xlibrgb.c;
/cvsroot/mozilla/gfx/src/xlibrgb/xlibrgb.c,v  <--  xlibrgb.c
new revision: 1.16; previous revision: 1.15
done
Checking in xlibrgb.h;
/cvsroot/mozilla/gfx/src/xlibrgb/xlibrgb.h,v  <--  xlibrgb.h
new revision: 1.11; previous revision: 1.10
done
cvs commit: Examining .
Checking in nsDeviceContextXP.cpp;
/cvsroot/mozilla/gfx/src/xprint/nsDeviceContextXP.cpp,v  <--  
nsDeviceContextXP.cpp
new revision: 1.8; previous revision: 1.7
done
Checking in nsXPrintContext.cpp;
/cvsroot/mozilla/gfx/src/xprint/nsXPrintContext.cpp,v  <--  nsXPrintContext.cpp
new revision: 1.7; previous revision: 1.6
done
Checking in nsXPrintContext.h;
/cvsroot/mozilla/gfx/src/xprint/nsXPrintContext.h,v  <--  nsXPrintContext.h
new revision: 1.7; previous revision: 1.6
done

Comment 74

[Masaki Katakai]

Roland,

Please give the instruncion how to verify.

Thank you.

Comment 75

[Roland Mainz]

Mhhh, you should see that images are put into it's original size into the
postscript job and scaled via postscript instructions instead of getting scaled
as bitmap and then added to the postscript job (which would be a nightmare, bit
picture + large scaling factor == insane large postscript job) ... 

Comment 76

[Roland Mainz]

Attachment: Testcase for verification

Comment 77

[Roland Mainz]

Comment on attachment 65788 [details]
Testcase for verification

Actually the testcase is useless.
Currently Xprint module uses the Xp image scaling API only for scaling the
images from the display framerbuffer resolution up to the printer device 
resolution, but it does not (like used in this example) scale images with
different scaling factors...

I filed bug 120967 to fix that...

Comment 78

[Masaki Katakai]

Can I mark this as dup of new bug 120967?

Comment 79

[Roland Mainz]

Masaki Katakai wrote:
> Can I mark this as dup of new bug 120967?

Uhm, no - because this is not a DUPlicate of bug 120967.
This one was about the issue that images were not scaled to the printer's
resolution - initial code didn't scale them, later code scaled them "manually"
on the application side (which resulted in insane lagr print jobs for
600/1200DPI printers).

The current code scales the images correctly using the |XpSetImageResolution()|
API (if the Xprt DDX supports this) to match the printer resolution - but
ignores when the destination size is bigger/smaller then the resolution-adjusted
size - the code then uses application-side scaling to fix that difference. I
filed bug 120967 to resolve that final scaling issue...