SLD: Add support to arithmetic expressions #5831
Conversation
|
At first glance this looks good. I kinda wish we could just extend the binding object to be more general to handle both a straight item and an expression but this work doesn't preclude that from happening at a later date. --Steve |
|
Do you have a feeling about how easy it would be to add this functionality in a more general sense? I think that should be done even if not part of your work. There are also at least one label property where this would be a nice addition (SIZE). --Steve |
|
Do you mean extending the binding object instead of having both |
|
That's what I was thinking but it's probably not worth the effort and you're right that expressions for things like colors don't make sense. Although enabling an expression binding would still need to be done in the mapfile parsing code (my second comment). I can take that piece on... |
| for(i=0; i<MS_LABEL_BINDING_LENGTH; i++) { | ||
| label->bindings[i].item = NULL; | ||
| label->bindings[i].index = -1; | ||
| msInitExpression(&(label->exprBindings[i])); |
There was a problem hiding this comment.
Thanks for pointing this out. I added msFreeExpression() in freeLabel() function.
| for(i=0; i<MS_STYLE_BINDING_LENGTH; i++) { | ||
| style->bindings[i].item = NULL; | ||
| style->bindings[i].index = -1; | ||
| msInitExpression(&(style->exprBindings[i])); |
There was a problem hiding this comment.
Thanks again. I added msFreeExpression() in freeStyle() function.
|
@sdlime As an exercise I am currently investigating how easy it would be to add expressions on label size. It has been a long time since I last played with lex and yacc :) |
I think we should add this capability across the board - I can help too. I think folks would be bummed if this nice addition was SLD-only. That should be a broader goal. I would include that commit in the pull request. Which got me wondering - how do things behave if an object already has an expression binding and then SLD defines another one? I would guess it would just free the old one and install the new one (essentially overwrite). Or this use case, there's an existing attribute binding and then SLD defines an expression binding? This might be a reason to have a single binding object? --Steve |
|
Commit is included in this pull request. I didn't check these specific use cases. They are worth a test in msautotest. An expression binding from SLD overwrites the one in MapFile and takes precedence over a simple binding from Mapfile. |
|
Thinking about it more doesn't SLD just create a bunch of new classes in a layer? (I'm not an SLD user) I mean, if classes are already defined then I'd guess they are removed and new ones created. |
|
I just checked the code: if a layer is defined both in MapFile and SLD, all classes from MapFile inside that layer are just removed and replaced by classes from SLD. |
|
I think we should go ahead and merge this code. Then we can work on a separate pull request that exposes the expression binding to more general mapfile processing (using your work in labels as a pattern). --Steve |
@jbo-ads - some excellent new features within this pull request. |
|
@geographika - If you don't mind waiting until september, I can add expression handling on STYLE->SIZE just like on LABEL->SIZE. |
|
Could merge now and add style->size as another pull request... |
@jbo-ads - whenever you get a chance works for me! As @sdlime says don't let it hold up merging this work. |
|
Unless someone objects, I'm considering pressing the 'Merge' button by tomorrow |
|
I guess we should squash the commits into a single one as the commit history in the PR is a bit messy ? |
|
It makes sense. Commit history was useful only for me while implementing the feature. |
|
Squashed and merged ! |
This pull request aims at adding support to arithmetic expressions in SLD documents read by MapServer. It implements the first section of MS RFC 124: Improving SLD Support in MapServer.