[Mapserver-dev] mapdraw.c

[Titus von der Malsburg]

Hi all,

I'm toying a little bit with Mapserver and Cairo.  While reading
mapdraw.c I found sections like this one

imageObj *msDrawMap(mapObj *map)
{
    ...
    else if( MS_RENDERER_GD(map->outputformat) )
    {
        image = msImageCreateGD(map->width, map->height,
                                map->outputformat, map->web.imagepath,
                                map->web.imageurl);
        if( image != NULL ) msImageInitGD( image, &map->imagecolor );
    }
    else if( MS_RENDERER_IMAGEMAP(map->outputformat) )
    {
        image = msImageCreateIM(map->width, map->height,
                                map->outputformat, map->web.imagepath,
                                map->web.imageurl);
        if( image != NULL ) msImageInitIM( image );
    }

    else if( MS_RENDERER_RAWDATA(map->outputformat) )
    {
        image = msImageCreate(map->width, map->height,
                              map->outputformat, map->web.imagepath,
                              map->web.imageurl);
    }
    ...
}

My problem with this is that during the rendering process these ifs and
the MS_RENDERER_GD macro must be called thousands of times.  But since
the outputformat wont change, it seems to me that this is redundant and
expensive (execution of the macro; the CPUs pipeline becomes often
invalid and has to be thrown away).

Wouldn't it be better to have something like this before the rendering
actually starts:

imageObj (*_msImageCreate)    (int, int, ....);
void     (*_msDrawLineSymbol) (symbolSetObj*, ...);
...

    ...
    else if( MS_RENDERER_GD(map->outputformat) )
    {
        _msImageCreate = msImageCreateGD;
        _msDrawLineSymbol = msDrawLineSymbolGD;
        ...
    }
    else if( MS_RENDERER_IMAGEMAP(map->outputformat) )
    {
        _msImageCreate = msImageCreateIM;
        _msDrawLineSymbol = msDrawLineSymbolIM;
        ...
    }
    ...

and later (where we formerly had the code above):

imageObj *msDrawMap(mapObj *map)
{
    
    ...
    image = _msImageCreate(map->width, map->height,
                           map->outputformat, map->web.imagepath,
                           map->web.imageurl);
    if( image != NULL ) _msImageInit( image, &map->imagecolor );
    ...
}


I guess this would make the code a bit faster, smaller, more readable
and maintainable.  For example it would be easier to add new
outputdrivers (that's what I'm doing).

What do you think?  I found some minor problems like the differences in
the argument structures of the functions, for example:
    msDrawLineSymbolGD(symbolset, image->img.gd, p, style, scalefactor);
vs.
    msDrawLineSymbolIM(symbolset, image, p, style, scalefactor);
But these could -- at least in those cases I've already seen -- easily
be solved.  In this case I would try to change msDrawLineSymbolGD
accordingly.

I'm new to Mapserver's code and therefore my comprehension of it is
limited.  Thus it would be great if you, the experienced mapserver
coders,  could give me some feedback before I start to burn my sparetime
only to find that my plan was flawed.

Thanks and
Cheers,
    Titus