[mapguide-internals] rfc60 ready for review

Trevor Wekel trevor_wekel at otxsystems.com
Fri Dec 11 14:19:11 EST 2009


Hi Tom,

Yes.  I will review the submission.  I believe Traian and Walt voiced concerns with the originally proposed solution.  It would be great if they had time to review it as well.  Hopefully the latest code has addressed most/all of the original concerns.

Thanks,
Trevor

-----Original Message-----
From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Tom Fukushima
Sent: December 11, 2009 10:45 AM
To: MapGuide Internals Mail List
Subject: RE: [mapguide-internals] rfc60 ready for review

Trevor,

Will you be reviewing any of this submission?

Tom

-----Original Message-----
From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of UV
Sent: December-11-09 9:48 AM
To: MapGuide Internals Mail List
Subject: Re: [mapguide-internals] rfc60 ready for review

Hi Tom,

1) We are using the patched 2.1 on our client website since may 2009 .
    So there are no more deadlines. However, right now the merging 
effort is the smallest as I already merged the branch with trunk.

    In this context a comment to the process:
    I still believe it to be wrong process for an open source project to 
have blocked this useful, transparent and working feature for 2.1 based 
on incompleteness of support of new stylization which
        a) nobody needed at that time which means no complex test cases 
to support development did exist
        b) might not even be so important as area styles are the only 
obvious visible features for color quantization errors
            (lets wait for a big map which is telling us otherwise)
        c) is not particular well documented/specified which in 
combination with missing complex test cases significantly increased the 
coding effort  (color quantization occurs on a complexity level which is 
very difficult to reach with unit level test cases)
        d) has no funding and little motivation to support it because we 
never needed it ourselves.

    Therefore adapting the RFC60 as it was and later create tickets for 
it after somebody exposed something missing with his maps (this way 
creating useful test cases) would be a much more appropriate and 
productive process to deal with such a feature which has been partially 
funded by a client as an external contribution in an open source 
project. Keywords: completeness, funding, motivation &  test cases
    I very much suggest to review your external contribution process 
again. At that time it was discouraging as the additional time spent 
dealing with extending the feature has artificially increased the effort 
to a multiple of the time solving our clients probem.
    Normally coding stops when the client problem is solved and nobody 
else pays for it.
    Dealing with external contributions like this might discourage 
people from contributing their patches to the shared code base, contrary 
to the open source idea. In this case we all were lucky because I had 
some spare time to invest.
------------------------------------------------------------------------------------------------
2) The revision merged from trunk is 4410 and can also be found in the 
MergeInfo property of  http://svn.osgeo.org/mapguide/sandbox/rfc60/MgDev
    The current revision on the rfc60 branch is 4411  - no changes yet!
------------------------------------------------------------------------------------------------
3)  In the Common code base I changed:        
        Map:                      add a member containing the string 
color list
        RenderingOptions:  move formatting of property to setter method 
(CTOR)
        AGGImageIO:       added color quantization code
        AGGRenderer:       pass the color palette through to AGGImageIo 
method
        DefaultStylizer:       added accessor and member for Symbolmanager

    In the Server code base I changed:        
        ServerManager.vcproj:     adding a number of include dirs
        MappingUtil:                     add code to extract strings 
from XML color fields into the map color table
        ServerRenderingService:   add parameter to control exection of 
color extraction code
                                                parse the string list to 
extract the colors (here any further expresssion evaluation can be 
easily added)
                                                (commented out: some 
code to let the complete tile fail when exception is caught in a layer
                                                 which makes sense for 
tiling,  see rfc64)
        ServerTileService:            comments and method extraction to 
help understand whats going on
                                                (copy & paste of code 
seems quite popular in same places which does not help)

This is all related to rfc60.



Tom Fukushima wrote:
> Hi UV,
>
> Since this is a large submission, please answer the following questions and requests for info, so we can figure out the logistics of getting the review done.
>
> 1) When do you need the review done by, do you have any pending hard deadlines?  MGOS 2.2 doesn't have a cut off that I know of yet so if all you want is for this to go into 2.2 we do have some time.
>
> 2) I noticed that you did a sync of your sandbox with trunk.  I think that the reviewers will just diff the sandbox with trunk to see what the changes are.  Which revision of trunk did you sync your sandbox with?
>
> 3) You say below that there were other changes that are not related to RFC 60.  There will be different reviewers for the RFC 60 part and the non-RFC 60 parts.  Please specify the list of files that were changed but were not related to RFC 60.
>
> Thanks
> Tom
>
>
>
> -----Original Message-----
> From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of UV
> Sent: December-09-09 5:57 PM
> To: MapGuide Internals Mail List
> Subject: [mapguide-internals] rfc60 ready for review
>
> In the sandbox on
>     http://svn.osgeo.org/mapguide/sandbox/rfc60/MgDev
> you can find a feature branch which is ready to be merged with trunk.
> This codetree includes code to deal with new stylization and lookup of 
> symboldefinitions in addition to the color lookup in the previous 
> stylization.
>
> *in MappingUtil (ExtractColors, FindColorInSymDefHelper)
>     strings are collected from the xml definition of the scaleRanges 
> which are describing stylization colors.
> *in ServerRenderingService (RenderMapInternal:777, ParseColorStrings)
>     after the Stylization the found strings are converted into colors 
> and passed on to the AGGRenderer
> *in AGGImageIo (ImageCopyForcePaletteGD)
>     a provided colorpalette is used to optimize the colorquantization 
> algorithm for the tile
>
> this code is only executed for PNG8 and GIF tiles.
>
> I further added comments for some existing methods and did some method 
> extraction while trying to understand the code.
>
> The interesting question is now if the colors found in the new 
> stylization can overlap tile boundaries in a visible way and therefore 
> need to be included into the color quantization optimization?
>
> In our map the long edges of the ocean areas with slightly different 
> colors where the only ones we could spot,
> whereas a slight color change of a line crossing a tile boundary is 
> quite hard to spot.
>
>
> Other than that, this feature is a MUST for everyone using colormapped 
> tiles as otherwise such map simply looks ugly .
> _______________________________________________
> mapguide-internals mailing list
> mapguide-internals at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/mapguide-internals
> _______________________________________________
> mapguide-internals mailing list
> mapguide-internals at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/mapguide-internals
>
>   

_______________________________________________
mapguide-internals mailing list
mapguide-internals at lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/mapguide-internals
_______________________________________________
mapguide-internals mailing list
mapguide-internals at lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/mapguide-internals



More information about the mapguide-internals mailing list