Cleaning up the paint API of KoShape

Inge Wallin inge at lysator.liu.se
Sun Jan 23 14:36:15 GMT 2011


There is a big wart on the KoShape API and that is the one for painting.  
Currently we have these two functions:

 virtual void paint(QPainter &painter, const KoViewConverter &converter) = 0;
 virtual void paintDecorations(QPainter &painter, 
                              const KoViewConverter &converter,
                              const KoCanvasBase *canvas);

The biggest problem is the KoViewConverter parameter.  There is no reason why 
this should be necessary. It should be the responsibility of the calling code 
to set up the painter so that the shape itself can just paint from (0, 0) to 
size() and that's it (if we for the moment ignore painting outside the shape).

Now we are forced to do things like 

 painter.fillRect(converter.normalToView(QRectF(QPointF(0.0,0.0), size())), 
background());

This is actually the way that is documented in the apidox.

The other problem is that having a separate paintDecorations() method can 
potentially introduce inefficiencies. My suggestion would instead be to 
introduce a new parameter to KoShape::paint(): PaintMode.  This could be an 
enum with the values { paintContentsOnly, paintDecorationsOnly, paintAll}, and 
maybe in the future more values. I can also imagine it as a bitmap with 
Contents and Decorations as bits. I'm open to suggestions here.

So the resulting API would become:

 typedef enum {
   paintContentsOnly,
   paintDecorationsOnly,
   paintAll
 } ShapePaintMode;

 virtual void paint(QPainter &painter, ShapePaintMode paintMode) = 0;

 ...or the alternative definition of ShapePaintMode.

If this is ok with the rest of you, I am willing to do the work.  My 
suggestion for the workflow to create as few interruptions as possible would 
be the following. I discussed this with Casper.

1. * Rename the current KoShape::paint() as KoShape::paintOld() and mark that 
as deprecated.
   * Change all the places that call KoShape::paint() to instead call 
paintOld().
   * At the same time, define a new KoShape::paint() with the new signature as 
defined above and create empty implementations in all shapes.
   * At the same time mark KoShape::paintDecorations() as deprecated.
   Step 1 is done in a branch, and should be possible to complete fairly 
quickly, in under a day at least.

2. Put the result of step 1 up for review and commit it.

3. Work in another branch (or perhaps the same) to implement the new paint() 
methods in all shapes.

4. Put this up for review and commit it.

5. Fix all the places in the code that call the paintOld() and 
paintDecoration() methods to instead use the new API.  This will probably take 
a bit longer and this can be merged with master also during the work, possibly 
after review(s).

6. Remove paintOld() and paintDecorations() from KoShape and all 
implementations from shapes.

Done.



More information about the calligra-devel mailing list