[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