Anonymous | Login | 04-16-2021 14:32 UTC |
Main | My View | View Issues | Change Log | Docs |
Viewing Issue Simple Details [ Jump to Notes ] | [ View Advanced ] [ Issue History ] [ Print ] | |||||||||||
ID | Category | Severity | Reproducibility | Date Submitted | Last Update | |||||||
0006303 | [Squeak] VM | minor | always | 03-05-07 18:59 | 01-09-08 03:38 | |||||||
Reporter | johnmci | View Status | public | |||||||||
Assigned To | ||||||||||||
Priority | normal | Resolution | open | |||||||||
Status | new | Product Version | 3.9 | |||||||||
Summary | 0006303: Matrix2x3Plugin does not consider negative numbers when rounding. | |||||||||||
Description |
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. |
|||||||||||
Additional Information | ||||||||||||
Attached Files |
![]() ![]() |
|||||||||||
|
![]() |
|||||||||||
SYSTEM WARNING: Creating default object from empty value
|
![]() |
|
(0010380 - 143 - 324 - 324 - 324 - 324 - 324) 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 - 106 - 106 - 106 - 106 - 106 - 106) 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 - 27 - 27 - 27 - 27 - 27 - 27) tim 12-27-07 23:10 |
Reminder sent to: johnmci Well, any thoughts on this? |
(0011600 - 130 - 130 - 130 - 130 - 130 - 130) 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 - 2054 - 2273 - 2273 - 2375 - 2375 - 2375) 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 - 208 - 247 - 247 - 247 - 247 - 247) 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 - 302 - 350 - 350 - 350 - 350 - 350) 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 - 1617 - 1833 - 1961 - 1961 - 1961 - 1961) 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 - 324 - 360 - 360 - 360 - 360 - 360) 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 |
Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
83 total queries executed. 51 unique queries executed. |