Opened 12 years ago

Closed 12 years ago

#1353 closed defect (fixed)

[raster] ST_SetRotation incorrect

Reported by: bnordgren Owned by: dzwarg
Priority: high Milestone: PostGIS 2.0.0
Component: raster Version: master
Keywords: Cc:

Description

SetRotation does not preserve the angle between the basis vectors. (The angle between ib and jb is different after St_SetRotation is called. Both axes should be rotated the same.)

This artifact is present even in the "expected results" of the rt_rotation test case. Clearly, the raster id=4 had both x and y skew until st_setrotation(rast,0) happened. The next time we see it (the first id=104), both skews had been set to zero (the expectation should be that y skew is zero, to align the i axis with the x axis; x skew will only become zero if the j axis is aligned with the y axis…which it most certainly is not.) The net effect is that ST_SetRotation() forced the axes to be orthogonal when they didn't start out that way.

The next time we see 104, ST_SetRotation(rast,pi/4) has been called on it. Now the i and j axes are both pointing in the same direction. Not only has the separation angle between the basis vectors been altered, it's one of the two illegal values (0 and pi). On this second trial, the axes went from being orthogonal to being parallel (illegal).

I remedied this in my working git repo with an implementation of DevWikiRealParameters and DevWikiAffineParameters, upon which st_rotation and st_setrotation now depend (in that repo). We can either pluck out the bits and pieces relevant to this issue or wait for the gen2 iterator…or you can reimplement the algorithms from scratch. Plucking out bits and pieces may be a bit sticky, since the new code is tested with CUnit tests, and the postgis repo lacks this infrastructure.

A revised "expected results" will be attached to this ticket. I checked this by hand with a calculator.

The commit which fixes this in my repo is: https://github.com/bnordgren/postgis/commit/06f9e65537e385f2adcb1b9ae166f50f5c7ad986

To browse source: https://github.com/bnordgren/postgis/tree/ri-gen2-svn

Attachments (1)

rt_rotation_expected (652 bytes ) - added by bnordgren 12 years ago.

Download all attachments as: .zip

Change History (8)

by bnordgren, 12 years ago

Attachment: rt_rotation_expected added

comment:1 by pracine, 12 years ago

Owner: changed from pracine to dzwarg

comment:2 by dzwarg, 12 years ago

Milestone: PostGIS 2.0.0PostGIS Raster Future
Priority: criticalhigh
Type: defectenhancement

The initial implementation of ST_SetRotation does not preserve the angle between the basis vectors by design. This is a future enhancement.

If you have a patch that includes the modifications, I can bring it in. It appears as though the github repo link has not been synced with postgis trunk for a couple months.

in reply to:  2 comment:3 by bnordgren, 12 years ago

Type: enhancementdefect

Replying to dzwarg:

The initial implementation of ST_SetRotation does not preserve the angle between the basis vectors by design. This is a future enhancement.

!!!!

Is there any reference to this design? If it's not rotating the entire plane around the origin, what is it doing?

Whatever it's intending to do, it is always an error to leave the raster with both i and j pointing in the same direction (or opposite directions). This happens even in your own test case, as indicated above. If you intend to defer preserving the angle between basis vectors until some future date, this artifact needs to be fixed now. But I can't help you with that because I'm not sure I understand what this function is trying to accomplish. I only know that the function produces a result which cannot be true under any circumstances.

If you have a patch that includes the modifications, I can bring it in. It appears as though the github repo link has not been synced with postgis trunk for a couple months.

At this point, I'm waiting for stability (a beta release to patch against). Also, if I recall correctly, my unit tests are written in CUnit, for consistency with liblwgeom. We'll either have to leave them behind or bring in the CUnit infrastructure to raster.

Again, I worked out all the equations for the future enhancement on the wiki, complete with figures.

comment:4 by bnordgren, 12 years ago

Milestone: PostGIS Raster FuturePostGIS 2.0.0

comment:5 by dzwarg, 12 years ago

Status: newassigned

Ah, I get it now. Sorry for the confusion.

I agree that leaving i and j pointing the same (or opposite) directions is an error.

I'll get on that one right away, but will defer the preservation of the angle between basis vectors. I will use this issue to resolve the problem with the expected results, and open another issue for an enhancement to preserve the angle between the basis vectors.

in reply to:  5 comment:6 by dzwarg, 12 years ago

Replying to dzwarg:

Ah, I get it now. Sorry for the confusion.

I agree that leaving i and j pointing the same (or opposite) directions is an error.

I'll get on that one right away, but will defer the preservation of the angle between basis vectors. I will use this issue to resolve the problem with the expected results, and open another issue for an enhancement to preserve the angle between the basis vectors.

Created ticket #1554 for basis vector preservation.

comment:7 by dzwarg, 12 years ago

Resolution: fixed
Status: assignedclosed

Fixed in r9109.

Note: See TracTickets for help on using tickets.