Opened 17 years ago
Closed 17 years ago
#2015 closed defect (fixed)
cellsize computations suffer from off-by-one error
Reported by: | sdlime | Owned by: | sdlime |
---|---|---|---|
Priority: | high | Milestone: | 5.0 release |
Component: | MapServer C Library | Version: | svn-trunk (development) |
Severity: | normal | Keywords: | |
Cc: | nsavard |
Description (last modified by )
The macro MS_CELLSIZE and function msAdjustExtent use width/height in their computations but should be using width-1 and height-1. The effect is subtle but needs to be fixed. Steve
Attachments (1)
Change History (14)
comment:2 by , 17 years ago
Maybe 5.0 is where we end this confusion by switching to a OWS extent interpretation and an UL pixel model. That way 0,0 in image coords will match minx,maxy in the extent (as it does now) but MapServer extents will match OWS extents. Steve
comment:3 by , 17 years ago
I would propose fixing 4.10 for issues with MS_CELLSIZE, msAdjustExtent and the OWS interfaces to use the current models for pixels and extents... Steve
comment:4 by , 17 years ago
Note, I've started a draft RFC (25) to get this cleaned up in 5.0... I'm especially interested in Frank's comments regarding the raster layer rendering code. Steve
comment:5 by , 17 years ago
Steve, Sorry for the lack of response. Your findings conflict with the mental model I had of how things were working so I feel I need to do some digging to review. I agree fixing this and making all assumptions clear for 5.0 is a good idea. But I am leery about possible implications for existing map files and mapscript scripts.
comment:6 by , 17 years ago
Please dig away. Note that the changes committed this morning to map.h and maputil.c are what is necessary to fix the cellsize computation issue I believe exists. These would need to be applied to 4.10 to make that right. In addition the WCS and perhaps the WMS client code would need tweaking to account for differences. The WMS server code appears to be accounting for the OWS/MapServer extent definition differences, as does the GDAL drawing code. I really don't think mapfiles or scripts would be impacted much. Most folks use MapServer for graphics and simple queries. A change like this to 4.10 would likely go unnoticed, but should be disclosed. Anyway, let me know what questions you might have. Steve
comment:7 by , 17 years ago
My proposed course of action would be to: For 4.10: - alter MS_CELLSIZE to divide by (d-1) in map.h - fix offet equations in msAdjustExtent to use width-1, height-1 - make sure OGC code differentiates between a MapServer extent and an OGC extent (e.g. fix bug 1983) That gets all the math right for standard MapServer and OGC interfaces. For 5.0: - propose using the OGC extent interpretation instead of the current one - perhaps consider a change to the pixel model Steve
comment:8 by , 17 years ago
Cc: | added |
---|---|
Description: | modified (diff) |
Milestone: | → 5.0 release |
Steve, are the changes in r5895 (2007-01-22) in MapServer 4.99 final?
I ask this because Norm noticed that the msautotests on GetFeatureInfo have a difference in the reported query coordinate location which I suspect may be related to this change, and we want to make sure that's correct before updating the tests.
comment:9 by , 17 years ago
I'm almost positive they are. I fixed that several months ago related to a WCS bug, but I will verify that when I get a chance later today and cofirm here.
Steve
comment:10 by , 17 years ago
I checked and there are 2 changes in 5.0 (answer to your question above is yes).
1) in msAdjustExtent() in maputil.c, we use image dimensions-1 when computing the new extent
2) in mapserver.h the MS_CELLSIZE macro used d-1 when computing cellsize
Those changes match the center pixel and extent model. The WCS code does the conversion now to convert between OWS bbox and a MapServer bbox (e.g. shrink by .5 a pixel). I believe the WMS server code does the same.
Two things to check.
1) Does the WMS layer code make that allowance? (e.g. expand by .5 pixel) 2) are the calls using MS_CELLSIZE ok?
For 2, I did a quick grep of the C code and can verify that the usage in mapserv.c, maptemplate.c, and maputil.c. In those cases a cellsize is being computed from a MapServer extent. There are calls to MS_CELLSIZE in mapwms.c and mapwmslayer.c that I have not checked.
Steve
comment:11 by , 17 years ago
Version: | 4.10 → svn-trunk (development) |
---|
comment:12 by , 17 years ago
I've found 2 other places that needed to be changed. Remember that with a pixel center to center extent model we need to use width-1 or height-1 when computing scale or cellsize. I discovered (via #2166) that the scale code needed that change (the comments were there saying that was the original way it was before things got confused in 4.x). See r6529 for the 2 changes I made. Things with regards to point/scale zooming for the CGI behave correctly now.
I still need to spend some time with the WMS code before candidate releases.
Steve
comment:13 by , 17 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
The WMS problem is contained in another bug so closing this one.
Steve