SYSTEM WARNING: Creating default object from empty value

Mantis - Squeak
Viewing Issue Advanced Details
6303 VM minor always 03-05-07 18:59 01-09-08 03:38
johnmci  
 
normal  
new 3.9  
open  
none    
none  
0006303: Matrix2x3Plugin does not consider negative numbers when rounding.
roundAndStoreResultPoint: nItemsToPop
    "Store the result of a previous operation.
    Fail if we cannot represent the result as SmallInteger"
    m23ResultX := m23ResultX + 0.5.
    m23ResultY := m23ResultY + 0.5.
related to 0005605assigned andreas Squeak oxymorons: MatrixTransform2x3 identity does not behave as an identiy transformation. 
child of 0006511new  Mother of all Morphic Graphical off-by-one/fencepost -error reports. 
 M23BugTests-M6303-wiz.1.cs [^] (3,745 bytes) 01-06-08 04:51
 Matrix2x3Plugi...ResultPoint.st [^] (966 bytes) 01-09-08 03:33

Notes
(0010380)
johnmci   
03-05-07 19:00   
rounded
    "Answer the integer nearest the receiver."

    self >= 0.0
        ifTrue: [^(self + 0.5) truncated]
        ifFalse: [^(self - 0.5) truncated]
(0011112)
tim   
09-12-07 00:28   
Reminder sent to: johnmci

Err, care to explain your note about rounded? What class? What relation to any changes in Matrix2x3Plugin?
(0011593)
tim   
12-27-07 23:10   
Reminder sent to: johnmci

Well, any thoughts on this?
(0011600)
johnmci   
12-28-07 05:04   
This became a non-issue in Sophie somewhere, likely the rome migration. Still I think it is a bug? if anyone confirm confirm that.
(0011623)
wiz   
01-05-08 01:27   
I think there is more than one bug here.

The method Matrix2x3Plugin>>roundAndStoreResultPoint:
lacks correctness when dealling with negative numbers.

The two routines that call this:
M23>>primitiveTransformPoint
M23>primitiveInvertPoint
would be prone to cumulative error problems because they are rounding the points (too) early and (too) often.
I suspect they were meant to be used ONLY with screen points scaled up by about 2**15. If used on unscaled points the error would be a significant. Especially on round trips.

I suspect in practice the scaling is not always used.

The store point routine calls
Interpreter>>makePointwithxValue: xValue yValue: yValue
which expects SmallInteger inputs.

The primitives seem incapable of returning points with float coordinates.

Finally screen rectangles will lose shape stability when origin and corner are rounded or truncated. What is wanted is probably floor. That would guarentee they are both shifted in the same direction.

The rounding and truncating are a poor mans floor when everything is assumed to be in the first quadrant. Transformation morphs usually insure this is not true.

See the picture examples in
0002453: [Bugs] What is the extent of a truncated rectangle?


What's needed are Transform and Invert primitives that preserve the precision of the results. Allowing appropriate flooring, compression or expansion to take place at the end of the chain of transformations guided by the user and the context in which the point is used.

I think Andreas originally intended that the chain of transformations take place by composing the M23s and then the final one is used at the end to transform points. Others who are not Andreas have probably writen code without that understanding. Indeed it is such an obsure point I wouldn't be surprised if even Andreas missed it from time to time.

I haven't had time to look for all the possible bugs that tie into this. My impression is there are many.

Yours in curiosity and service, --Jerome Peace
(0011628)
nicolas cellier   
01-05-08 20:15   
I confirm what john said:

-2.7 + 0.5 -> -2.2 -> -2 (once implicitly cast by C from double to int)
-2.7 rounded -> -3

Of course, that would not solve all these early rounding problems Jerome told about.
(0011629)
wiz   
01-06-08 00:11   
I'm a firm believer in partial progress.

Until the deeper problems can be tackled, correcting the rounding of the negative numbers would be an improvement.

That would be a good increment.

And if someone wanted to write an non-casting point maker that would be another good step.

Cheers -Jer
(0011631)
wiz   
01-06-08 04:58   
From the preamble:

Change Set: M23BugTests-wiz
Date: 5 January 2008
Author: (wiz) Jerome Peace

wiz 1/5/2008 23:28

This is the start of tests for MatrixTransform2x3 bugs.
Currently we are testing the bug in roundoff of negative numbers by the m23primitives for transformPoint and invertPoint.

The invariant tested is that the result of calling a transform should be equivalent to the result that would be given if the primitive fails.

Also currently the only suspect being tested is the identity transform.

This fails because of the bug mentioned in mantis 6303
http://bugs.squeak.org/view.php?id=6303. [^]

To do: a test for the Matrix plugin needs to be made. The test will trivially pass if the module is absent.

The list of suspects will need to be widened once a repair makes it pass the identiy case.

Other notes: I have crafted the tests so that upon failure 'rejects explore' will give infomation as to which suspects failed and with which test points.

Doing this currently shows the one suspect fails for all points containing at least one negative coordinate.

====

Widening the list of suspects shows that some of the scaling transformation can pass the tests. This makes sense because scaling by a large enough amount makes the roundoff error less important.

Anyway the routine is a good initial test which can be improved.

I am currently running on my old iMac under 9.1 using a classic os vm. So unless John goes back to supporting that platform I won't be the first one to see this test turn green.

Hth,

Yours in curiosity and service, --Jerome Peace
(0011644)
wiz   
01-09-08 03:38   
Matrix2x3Plugi...ResultPoint.st uploaded.

This is a swing at just the specific patch that could make the test pass.
I say could because I am not set up to test it or even use it. ( My mac has no facility for compiling vm's or c code and I am not experienced there (yet).
Consider this a starting point.

Cheers, -Jer