NSImage drawing (was Re: Problem with Gorm)

[Eric Wasylishen]

Hi Fred.

> I had a quick look into these new methods and didn't like too much what I saw 
> there.

I agree. It's a real mess.

The positive thing is that most of the rest of NSImage is pretty tidy, and all 
drawing goes to -drawInRect:..., so we just need to fix that one method :-).

> We have a lot of code duplication between these two methods, but we also have 
> differences there which I don't understand. Why do we use scaling for the 
> native drawing but not for the gui drawing?

Not sure, what do you mean by scaling? btw, my ImageSelectionTest in GSTest 
currently lacks a test of the effect of setting a transformation matrix / a 
NSView bounds transform before drawing an image, and that needs to be added. 
The test should be renamed to ImageDrawing-test since it's turning in to a 
general test of image drawing.

> Anyway, the code in native drawing should not check for isDrawingToScreen. 
> When drawing to a PDF or PostScript surface we should be able to composite or 
> dissolve, we just need to package that up correctly as backend operations. 
> And of course need to have the PS and PDF context report back that they are 
> able to support this.

Agreed, the code branch that checks isDrawingToScreen should be removed 
entirely.

> I also noticed that the new code wont fill the cached image rep with the 
> image background colour. Why isn't this done and does this match the Cocoa 
> behaviour?

I don't know - that needs to be tested and added to the image test in GSTest. 
On my todo-list is making GSTest run on OS X so we can verify the same output.

> Why does the old code (guiDrawInRect:...) use PSgsave and PSgrestore around 
> the cache drawing? We wont do any other operations on the cache and don't 
> need to restore the old state.

Not sure..

> I think that we should start a discussion about this whole drawing code. 
> Hopefully we end up with a deeper understanding of what should happen here 
> and will then be able to package it up with a nice and clean call to the 
> backend and three or four (almost?) correct implementations for this in 
> GSContext, GSStreamContext and GSCairoContext. But then I may be completely 
> wrong here, as I really don't understand much of this new code.

What I would like to do is: 

- get rid of -guiDrawInRect entirely.
- reinstate caching since currently there is no caching whatsoever.
- possibly switch to the new 
drawInRect:fromRect:operation:fraction:respectFlipped:hints: method 

Our goal should be for the following cairo calls to be used when drawing an 
uncached image:

cairo_surface_t *src = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, [rep 
pixelsWide], [rep pixelsHigh]);
// copy the ARGB data in NSBitmapImageRep to the src image surface in BGRA 
format
cairo_t *cr = cairo_create(dest);
// set the transformation matrix and clipping rect on cr to refect the source 
rect and dest rect
cairo_set_source_surface(cr, src, 0, 0);
cairo_paint(cr);
cairo_destroy(cr);
cairo_surface_destroy(src);

That's it, I think.  To do caching, the code is the same except the 'src' 
surface is created as an xlib surface so it's stored on the X server, and the 
surface is kept around and reused, so the copying from NSBitmapImageRep to 
cairo only happens once as long as the cached surface is kept alive.

Cheers,
Eric