Context tests broken (e.g. REQUIRES)

[sdlime]

Older versions of MapServer drawing code actually manipulated LAYER status if 
scale tests failed. Now with the function msLayerIsVisible the status is not 
changed. This is a good thing with MapScript etc... Problem is that REQUIRES is 
not catching layers turned off by scaling. msEvalContext needs to be reworked 
to use msLayerIsVisible itself. Problem is that msLayerIsVisible calls 
msEvalContext and that can lead to an infinite loop if 2 context expressions 
refer to each other, for example:

LAYER
  NAME PARKS
  REQUIRES "![DOQS]"
  ...
END

LAYER
  NAME DOQS
  REQUIRES "![PARKS]"
  ...
END

Never mind that it doesn't make sense, it will cause a loop though. To know if 
PARKS is visible I need to know if DOQS are visible and so on...

Do we accept this as a possiblity or figure out a way to stop it...

Steve

[dmorissette]

Fixing this would probably help solving bug 428 too.

I'd say we produce an error in case of recursive loops like this. The big
question is how to track msEvalContext calls to prevent recursive loops?

[sdlime]

I think we'll need a msValidateContexts() function that is run in 
msPrepareImage and the msQuery... functions. It can step through each layer 
and if a context is defined it can check each layer in the context (and 
referenced layers) for a reference to the layer being checked. I think this 
check needs to be made outside of msLayerIsVisible() the the highest level 
drawing and query functions seem to be the place. If too much recusion is 
found we'd throw an error. What do you guys think?

Steve

[dmorissette]

I don't see any problem (I'm not even sure to fully understand what you mean but
maybe I just don't need to understand).

So where would msValidateContext() store the results? Would it update the
layer->status? I'm new to all this so maybe this is a stupid question, but
what's the benefit of this over the old approach that was setting the
layer->status as well? Hopefully we would be able to use this to solve the
legend issue in bug 428?

[sdlime]

It would just be a hoop to jump through. That is, if we pass that test it's ok
to call the visibility function. We can rely on the visibility function then in
context evaluaton and in the layer code. There still may be a case where layers
are created dynamically after msPrepareImage, that may be a reason to set this
up as a layer level function msLayerValidateContext instead. One way or another
it's just a test a layer would have to pass, kinda like the visibility function.

Steve

[sdlime]

I've got the context validation function written and in place in msPrepareImage,
and msEvalContext now uses msLayerIsVisible. The problem is that the
msLayerIsVisible function is not being used everywhere it could be, specifically
the legending (image and templated) code, and in drawing a query map. I'll fix
those as part of this bug later tonite unless there are objections soon.

Steve

[sdlime]

I've committed this fix and updated msDrawMap and the legend creation functions
in maptemplate.c and maplegend.c to check for recursion problems. It is still
possible to by-pass this test in MapScript by creating layers after calling the
prepareImage method. However, that leads to other problems such as missing
symbol/label scaling coeficients so should be considered bad programming
practice. The msEvalContext function now calls msLayerIsVisible and should be
much more robust. Marking as fixed, but this is an area we'll want to watch. 

Note that I filed seperate bugs related to msDrawQueryMap and msDrawLegend.

Steve