Add PROCESSING FORCE_DRAW_LABEL_CACHE layer-level directive #5100

Merged
merged 2 commits into from Jun 23, 2015

Conversation

Projects
None yet
3 participants
@dmorissette
Contributor

dmorissette commented May 12, 2015

It can sometimes be useful to flush the labelcache in the middle of map rendering for various reasons.

This pull request introduces a PROCESSING FORCE_DRAW_LABEL_CACHE directive that can be added to a layer to force rendering and flushing the labelcache after the layer is done rendering. This would typically be used on the last layer of a group of layers that should be treated as an independent group with respect to the label cache, and before starting the rendering of another set of independent layers. Setting this directive to any value triggers this behavior.

e.g.

LAYER
   ...
   PROCESSING "FORCE_DRAW_LABEL_CACHE=1"
   ...
END
@dmorissette

This comment has been minimized.

Show comment
Hide comment
@dmorissette

dmorissette May 13, 2015

Contributor

@tbonfort, are you okay with me merging this into branch-7-0?

Contributor

dmorissette commented May 13, 2015

@tbonfort, are you okay with me merging this into branch-7-0?

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort May 13, 2015

Member

I'm a bit reluctant to mess with the labelcache as it's really not meant to be rendered multiple times, plus the call to init/freelabelcache would invalidate the record-keeping of the labels that have actually been rendered and you'd end up with overwritten labels.
What's the use-case for this?
Why can you not use the PRIORITY concept to obtain the same result, i.e.:

layer
 group "1"
 label
  priority 10
 end
end
layer
 group "1"
 label
  priority 10
 end
end
layer
 group "2"
 label
  priority 1
 end
end

which should end up as the same result as

layer
 group "1"
 label
 end
end
layer
 group "1"
 processing "FORCE_DRAW_LABEL_CACHE=1"
 label
 end
end
layer
 group "2"
 label
 end
end
Member

tbonfort commented May 13, 2015

I'm a bit reluctant to mess with the labelcache as it's really not meant to be rendered multiple times, plus the call to init/freelabelcache would invalidate the record-keeping of the labels that have actually been rendered and you'd end up with overwritten labels.
What's the use-case for this?
Why can you not use the PRIORITY concept to obtain the same result, i.e.:

layer
 group "1"
 label
  priority 10
 end
end
layer
 group "1"
 label
  priority 10
 end
end
layer
 group "2"
 label
  priority 1
 end
end

which should end up as the same result as

layer
 group "1"
 label
 end
end
layer
 group "1"
 processing "FORCE_DRAW_LABEL_CACHE=1"
 label
 end
end
layer
 group "2"
 label
 end
end
@dmorissette

This comment has been minimized.

Show comment
Hide comment
@dmorissette

dmorissette May 13, 2015

Contributor

The use case is overlaying datasets from different sources in the same map wiht a mask in-between the two sources. For instance, imagine a country-level map using Natural Earth data, on top of which you overlay some street level data for a given city.

As you zoom in, you want to display the street level data for the city area, and stick with the Natural Earth data for the rest of the country where you have no better data, just to provide some context when you pan to the edge of the city.

A simple way to do that is to draw the Natural Earth layers first, then draw a mask to hide the city area, and then draw the street level data on top of the mask for the city streets. But if you don't flush the label cache in-between, then the NaturalEarth labels end up showing out of context on top of the mask and of the street data.

Of course the proper way to do this would be to process the data to merge the two datasets, but this is not always an option for other use cases where each dataset that is overlayed needs to remain unchanged.

Makes sense? We can try to come up with a picture to illustrate the use case of the explanation is not clear enough.

Contributor

dmorissette commented May 13, 2015

The use case is overlaying datasets from different sources in the same map wiht a mask in-between the two sources. For instance, imagine a country-level map using Natural Earth data, on top of which you overlay some street level data for a given city.

As you zoom in, you want to display the street level data for the city area, and stick with the Natural Earth data for the rest of the country where you have no better data, just to provide some context when you pan to the edge of the city.

A simple way to do that is to draw the Natural Earth layers first, then draw a mask to hide the city area, and then draw the street level data on top of the mask for the city streets. But if you don't flush the label cache in-between, then the NaturalEarth labels end up showing out of context on top of the mask and of the street data.

Of course the proper way to do this would be to process the data to merge the two datasets, but this is not always an option for other use cases where each dataset that is overlayed needs to remain unchanged.

Makes sense? We can try to come up with a picture to illustrate the use case of the explanation is not clear enough.

@dmorissette

This comment has been minimized.

Show comment
Hide comment
@dmorissette

dmorissette Jun 1, 2015

Contributor

@sdlime, @tbonfort, any objection for me to merge in light of the explanation above?

Contributor

dmorissette commented Jun 1, 2015

@sdlime, @tbonfort, any objection for me to merge in light of the explanation above?

@sdlime

This comment has been minimized.

Show comment
Hide comment
@sdlime

sdlime Jun 1, 2015

Member

I'm ok with this I guess in principle, but do have a few suggested changes. I have a similar need where there's a mask and I'd like to only allow some labels to potentially be drawn on top of it. I don't see how this would solve that problem without a little more control since the layer that I want labels drawn after the mask is drawn fairly early but with high label priority. What about flushing the cache up to a certain priority, that is, the value you set matters.

processing "FORCE_DRAW_LABEL_CACHE=8"

Would render all labels up to level 8... Of course that limits the chance the things I care about in levels 9 or 10 might not have space any more.

Also, does this really flush the cache? If so then how are collisions with already placed labels handled, or aren't they? If this is truly starting over then maybe the key should be FLUSH_LABEL_CACHE?

Steve

Member

sdlime commented Jun 1, 2015

I'm ok with this I guess in principle, but do have a few suggested changes. I have a similar need where there's a mask and I'd like to only allow some labels to potentially be drawn on top of it. I don't see how this would solve that problem without a little more control since the layer that I want labels drawn after the mask is drawn fairly early but with high label priority. What about flushing the cache up to a certain priority, that is, the value you set matters.

processing "FORCE_DRAW_LABEL_CACHE=8"

Would render all labels up to level 8... Of course that limits the chance the things I care about in levels 9 or 10 might not have space any more.

Also, does this really flush the cache? If so then how are collisions with already placed labels handled, or aren't they? If this is truly starting over then maybe the key should be FLUSH_LABEL_CACHE?

Steve

@dmorissette

This comment has been minimized.

Show comment
Hide comment
@dmorissette

dmorissette Jun 1, 2015

Contributor

Yes, it really flushes the cache, so the key could be called FLUSH_LABEL_CACHE if that makes more sense. In our use case there is no need for handling collisions with already placed labels, since the mask hides the labels under the mask and the rest of the layers drawn after this call are only drawn on top of the mask.

Contributor

dmorissette commented Jun 1, 2015

Yes, it really flushes the cache, so the key could be called FLUSH_LABEL_CACHE if that makes more sense. In our use case there is no need for handling collisions with already placed labels, since the mask hides the labels under the mask and the rest of the layers drawn after this call are only drawn on top of the mask.

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Jun 4, 2015

Member

I'm -0 on integrating this as it is hacky, resolves a very particular use case, and fiddles with the labelcache component that is already very brittle and therefore compromises/complexifies any future development in that area.
If I understand correctly, the same ouput could be obtained by calling a client wms layer of the naturalearth mapfile, and then painting over it with the mask and osm data.

Member

tbonfort commented Jun 4, 2015

I'm -0 on integrating this as it is hacky, resolves a very particular use case, and fiddles with the labelcache component that is already very brittle and therefore compromises/complexifies any future development in that area.
If I understand correctly, the same ouput could be obtained by calling a client wms layer of the naturalearth mapfile, and then painting over it with the mask and osm data.

@dmorissette

This comment has been minimized.

Show comment
Hide comment
@dmorissette

dmorissette Jun 4, 2015

Contributor

Yes, the same output can be achieved with WMS cascading, and this is actually the way that this was handled before. However, cascading WMS results in a significant performance hit (process overhead + latency), and this pull request was exactly to get away from the WMS cascading and resolve the performance issue.

The Natural Earth use case was only provided as a simple example to illustrate the need. The real use case we are dealing with is S57 nautical charts. Those maps are composed of 6 "bands" of vector layers with corresponding scales. They are listed here: http://www.nauticalcharts.noaa.gov/csdl/ctp/encdirect_help.htm#naming

Let's look at two of the bands to illustrate the need: The "General" band contains coverage for the whole water area and some coastline features relevant only to the General scale. The next level which is "Coastline" contains only coverage for the coastline, at a more detailed level, so as you zoom in to coastline scale near the edge of the coastline or in the middle of the sea, you lose all context unless you keep combining all layers as you zoom in. Using WMS cascading, we end up launching 6 WMS client requests at the most zoomed in level, which results in awful performance, and this pull request makes those maps fly. The alternative without this pull request would involve a significant amount of processing of the data to generate 6 versions of the merged datasets.

Since the pull request simply flushes the label cache completely and does not really fiddle with it, I am not too worried about complexity or side-effects and am willing to assist in dealing with issues if/when they arise. Two clear limitations of this is that (1) some long labels located under the mask could be truncated by the edge of the mask, and that (2) there could be undetected label collisions outside the mask between layers draw before/after the mask. These limitations should be included in the docs for this feature to manage users expectations and avoid complaints.

Here are two images showing examples, the problematic labels are shown in red in the first image, and the second image uses the FORCE_DRAW_LABEL_CACHE directive to deal with the issue:

example_with_problem_labels_in_red

example_with_force_label_cache

Contributor

dmorissette commented Jun 4, 2015

Yes, the same output can be achieved with WMS cascading, and this is actually the way that this was handled before. However, cascading WMS results in a significant performance hit (process overhead + latency), and this pull request was exactly to get away from the WMS cascading and resolve the performance issue.

The Natural Earth use case was only provided as a simple example to illustrate the need. The real use case we are dealing with is S57 nautical charts. Those maps are composed of 6 "bands" of vector layers with corresponding scales. They are listed here: http://www.nauticalcharts.noaa.gov/csdl/ctp/encdirect_help.htm#naming

Let's look at two of the bands to illustrate the need: The "General" band contains coverage for the whole water area and some coastline features relevant only to the General scale. The next level which is "Coastline" contains only coverage for the coastline, at a more detailed level, so as you zoom in to coastline scale near the edge of the coastline or in the middle of the sea, you lose all context unless you keep combining all layers as you zoom in. Using WMS cascading, we end up launching 6 WMS client requests at the most zoomed in level, which results in awful performance, and this pull request makes those maps fly. The alternative without this pull request would involve a significant amount of processing of the data to generate 6 versions of the merged datasets.

Since the pull request simply flushes the label cache completely and does not really fiddle with it, I am not too worried about complexity or side-effects and am willing to assist in dealing with issues if/when they arise. Two clear limitations of this is that (1) some long labels located under the mask could be truncated by the edge of the mask, and that (2) there could be undetected label collisions outside the mask between layers draw before/after the mask. These limitations should be included in the docs for this feature to manage users expectations and avoid complaints.

Here are two images showing examples, the problematic labels are shown in red in the first image, and the second image uses the FORCE_DRAW_LABEL_CACHE directive to deal with the issue:

example_with_problem_labels_in_red

example_with_force_label_cache

@dmorissette

This comment has been minimized.

Show comment
Hide comment
@dmorissette

dmorissette Jun 4, 2015

Contributor

Um... I'm wondering if using MINDISTANCE would allow us to address this duplicate label issue. We'll make some tests.

Contributor

dmorissette commented Jun 4, 2015

Um... I'm wondering if using MINDISTANCE would allow us to address this duplicate label issue. We'll make some tests.

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Jun 4, 2015

Member

Mindistance only works for labels coming from the same layer + class

Member

tbonfort commented Jun 4, 2015

Mindistance only works for labels coming from the same layer + class

@dmorissette

This comment has been minimized.

Show comment
Hide comment
@dmorissette

dmorissette Jun 4, 2015

Contributor

That's what I thought but I wasn't sure. Thanks for confirming it.

Contributor

dmorissette commented Jun 4, 2015

That's what I thought but I wasn't sure. Thanks for confirming it.

@sdlime

This comment has been minimized.

Show comment
Hide comment
@sdlime

sdlime Jun 4, 2015

Member

If this is added then I'd go with the FLUSH_LABEL_CACHE name. It's more descriptive IMHO.

Member

sdlime commented Jun 4, 2015

If this is added then I'd go with the FLUSH_LABEL_CACHE name. It's more descriptive IMHO.

@dmorissette

This comment has been minimized.

Show comment
Hide comment
@dmorissette

dmorissette Jun 4, 2015

Contributor

or maybe FORCE_DRAW_LABEL_CACHE=FLUSH which would keep the door open for eventually adding the kind of stuff (FORCE_DRAW_LABEL_CACHE=8) that you mentioned above? However the potential for side-effects of this other option would be greater than just a straight draw/flush operation I think.

Contributor

dmorissette commented Jun 4, 2015

or maybe FORCE_DRAW_LABEL_CACHE=FLUSH which would keep the door open for eventually adding the kind of stuff (FORCE_DRAW_LABEL_CACHE=8) that you mentioned above? However the potential for side-effects of this other option would be greater than just a straight draw/flush operation I think.

@sdlime

This comment has been minimized.

Show comment
Hide comment
@sdlime

sdlime Jun 4, 2015

Member

I'm cool with that, I wanted to leave the door open. I don't know the label cache code well since it was re-written but intuitively it seems like doing something like draw labels, render an intermediate layer. draw more labels would work where the second label draw would be able to use placements from the first.

Member

sdlime commented Jun 4, 2015

I'm cool with that, I wanted to leave the door open. I don't know the label cache code well since it was re-written but intuitively it seems like doing something like draw labels, render an intermediate layer. draw more labels would work where the second label draw would be able to use placements from the first.

dmorissette added a commit that referenced this pull request Jun 23, 2015

Merge pull request #5100 from dmorissette/force-labelcache
Add PROCESSING FORCE_DRAW_LABEL_CACHE=FLUSH layer-level directive to allow flushing label cache between layer draws (#5100)

@dmorissette dmorissette merged commit 3ffbb26 into mapserver:branch-7-0 Jun 23, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment