New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MaxFeatures (for drawing) support lost at 6.2 #4730

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@tbonfort
Member

tbonfort commented Aug 20, 2013

The layer MAXFEATURES parameter was a means of limiting the number of features drawn. The parameter also was used for limiting query results- kind of a mess. Things were refactored in 6.2 I believe (TODO: find ticket number) and the drawing support seems to be gone...Look specifically at the lakes in the following maps.

Before: 6.0

After: 6.2

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Aug 13, 2013

Member

c.f. #3739 ?

Member

tbonfort commented Aug 13, 2013

c.f. #3739 ?

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Aug 13, 2013

Member

it would seem that int msLayerGetMaxFeaturesToDraw(layerObj *layer, outputFormatObj *format) isn't checking for layer->maxfeatures

Member

tbonfort commented Aug 13, 2013

it would seem that int msLayerGetMaxFeaturesToDraw(layerObj *layer, outputFormatObj *format) isn't checking for layer->maxfeatures

tbonfort added a commit to mapserver/msautotest_DEPRECATED that referenced this pull request Aug 20, 2013

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Aug 20, 2013

Member

committed to master in a0ef44a along with a couple of tests

Member

tbonfort commented Aug 20, 2013

committed to master in a0ef44a along with a couple of tests

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Aug 20, 2013

Member

@sdlime I'll let you close this one if you can confirm the fix.

Member

tbonfort commented Aug 20, 2013

@sdlime I'll let you close this one if you can confirm the fix.

@sdlime

This comment has been minimized.

Show comment
Hide comment
@sdlime

sdlime Aug 20, 2013

Member

Maybe I should just update my mapfile to use the metadata (maxfeaturestodraw) and we revert your changes.

Steve

Member

sdlime commented Aug 20, 2013

Maybe I should just update my mapfile to use the metadata (maxfeaturestodraw) and we revert your changes.

Steve

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Aug 20, 2013

Member

is your layer->maxfeatures set to 0 in your mapfile ? As to the check for layer->maxfeatures, the documentation is pretty clear that the check also happens when drawing, not only for WFS requests.

Member

tbonfort commented Aug 20, 2013

is your layer->maxfeatures set to 0 in your mapfile ? As to the check for layer->maxfeatures, the documentation is pretty clear that the check also happens when drawing, not only for WFS requests.

@sdlime

This comment has been minimized.

Show comment
Hide comment
@sdlime

sdlime Aug 20, 2013

Member

MaxFeatures is set to 100. Metadata doesn't work either. --Steve

Member

sdlime commented Aug 20, 2013

MaxFeatures is set to 100. Metadata doesn't work either. --Steve

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Aug 20, 2013

Member

Ok, I'll have a look

Member

tbonfort commented Aug 20, 2013

Ok, I'll have a look

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Aug 20, 2013

Member

@sdlime I've traced the code with some debugging statements and it seems to be working as expected for me. The test added in mapserver/msautotest_DEPRECATED@42965a3 also seems to point that features are rendered, and are limited to the maxfeature/maxfeaturestodraw parameters (map->web->metadata->maxfeaturestodraw is not tested).

Member

tbonfort commented Aug 20, 2013

@sdlime I've traced the code with some debugging statements and it seems to be working as expected for me. The test added in mapserver/msautotest_DEPRECATED@42965a3 also seems to point that features are rendered, and are limited to the maxfeature/maxfeaturestodraw parameters (map->web->metadata->maxfeaturestodraw is not tested).

@sdlime

This comment has been minimized.

Show comment
Hide comment
@sdlime

sdlime Aug 20, 2013

Member

Ok, this will sound weird but what I'm seeing is the difference the order a shapefile is being processed or appears to be processed. Maxfeatures just surfaces the change. The shapefile I'm using to draw 100 largest lakes is ordered from smallest to largest.

Maxfeatures used to be pretty much a shapefile only thing and if you look at mapshape.c in 6.0 you'll still see that attribute referenced in msSHPLayerWhichShapes(). That's where the change is. There's a block of code in 6.0 that actually modifies the bit arrary leaving at most maxfeatures on, but it leaves the last maxfeatures on. There must have been reason it was done this way and probably had with drawing order when ordering by size, that is, drawing the n largest of something but not wanting to the big stuff to obscure the little stuff. Pretty specific use case I guess. So this is a regression although it seems I'm the only one to notice... ;-)

Your patch is correct, I just need to deal with the change on my side.

Steve

Member

sdlime commented Aug 20, 2013

Ok, this will sound weird but what I'm seeing is the difference the order a shapefile is being processed or appears to be processed. Maxfeatures just surfaces the change. The shapefile I'm using to draw 100 largest lakes is ordered from smallest to largest.

Maxfeatures used to be pretty much a shapefile only thing and if you look at mapshape.c in 6.0 you'll still see that attribute referenced in msSHPLayerWhichShapes(). That's where the change is. There's a block of code in 6.0 that actually modifies the bit arrary leaving at most maxfeatures on, but it leaves the last maxfeatures on. There must have been reason it was done this way and probably had with drawing order when ordering by size, that is, drawing the n largest of something but not wanting to the big stuff to obscure the little stuff. Pretty specific use case I guess. So this is a regression although it seems I'm the only one to notice... ;-)

Your patch is correct, I just need to deal with the change on my side.

Steve

@sdlime sdlime closed this Aug 20, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment