Ticket UUID: | 610a73a1792310b54fd630bfba937df5b9c449ee | |||
Title: | Canvas widget handles pixel objects incorrectly in Tk 9.0 | |||
Type: | Bug | Version: | 8.6 | |
Submitter: | marc_culler | Created on: | 2024-08-24 22:43:45 | |
Subsystem: | 04. Canvas Basics | Assigned To: | jan.nijtmans | |
Priority: | 5 Medium | Severity: | Important | |
Status: | Closed | Last Modified: | 2024-10-05 19:49:24 | |
Resolution: | Fixed | Closed By: | jan.nijtmans | |
Closed on: | 2024-10-05 19:49:24 | |||
Description: |
Here is a wish 9.0 session: % pack [canvas .c] % pack [entry .e] % .c configure -borderwidth 10p % .c cget -borderwidth 13 % .e configure -borderwidth 10p % .e cget -borderwidth 10p I think that the entry is correct and the canvas is wrong. The size of 10p in pixels depends on the screen resolution, which can change. A borderwidth value of 10p should be stored as a Tcl_Obj of type "pixels", and that is what cget should return. The entry does that, but the canvas does not. | |||
User Comments: |
jan.nijtmans added on 2024-10-05 19:49:24:
Fix should be complete in [b519b0b9bb6ec777] now. jan.nijtmans added on 2024-09-27 13:51:38: > The change in checkin [24db3e18] breaks our use of Tk. Because of this, I don't think it's a good idea to backport those fixes to Tk 8.7. Its better not break existing applications in a minor release, even though it is the 'correct' thing to do. griffin added on 2024-09-21 14:22:48: VHDL has a class of types called a Physical Type. These types declare a base unit and a number of other units as a multiple of the base unit. Maths can include physical values, and the value is converted to base units. Here’s an example: type DISTANCE is range 0 to 1E5 units um; -- micrometer mm = 1000 um; -- millimeter in_a = 25400 um; -- inch end units DISTANCE; variable Dis1, Dis2 : DISTANCE; Dis1 := 28 mm; Dis2 := 2 in_a - 1 mm; if Dis1 < Dis2 then ... I know Tcl does not have a concept of “type”, but maybe GetNumberFrom api calls could possibly lookup suffix definition to find a scaling factor and apply it. (Time is the only predefined physical type in the language) Just a thought. chw added on 2024-09-21 05:40:34: Marc, let me hallucinate even further: in a distant Tcl land of the future the AI enhanced expression elevator (AI3E) can convert colors expressed in arbitrary spaces (be it RGB or HSV) to e.g. speeds in rarely used units, e.g. Angstrøm per week, or keystrokes to scrollwheel miles per gallon. Just kidding! marc_culler (claiming to be Marc Culler) added on 2024-09-21 02:52:56: I could imagine a world in which the expression evaluator would know to convert a TclObj of type "pixels" to its current numerical value and use that value in evaluating the expression. But I suspect that would be difficult to implement due to the fact that the "pixels" type is registered by Tk, not within Tcl. marc_culler (claiming to be Marc Culler) added on 2024-09-20 23:22:17: I agree with Jan that the new behavior is correct. Screen coordinates, and distances between points given by screen coordinates, are numbers. A pixel value, like 10p, is not a number. It represents a numerical value which is meant to be interpreted as a screen distance, but that numerical value depends on the resolution of the screen, and the resolution of the screen can change at any time. The commands winfo pixels and winfo fpixels are what the Tk language provides for converting a pixel value to the number which that pixel value represents at the current time. In Tk 9, a pixel value is modeled by a TclObj of type "pixels". Since that object type did not exist in Tk 8, the language could not fully support the concept of a screen distance which is independent of the screen resolution. But now, in Tk 9, it can. And I think that it should. So we should just roll up our pants. chw added on 2024-09-20 20:54:53: Indeed Don, so 20+ years we used to write $w cget -width and now we have to spell winfo pixels $w [$w cget -width] Modern times. After my second glass of wine, I guess we managed to double the code. Progress, cheers, Christian "I grow old... I grow old... I shall wear the bottoms of my trousers rolled." -- T.S. Elliot dgp added on 2024-09-20 20:17:14: For the sake of anyone who lands here searching for how to deal with this, another tool to consider is the command [winfo pixels] chw added on 2024-09-20 20:06:25: Is it time for TIPery? What about $widget cget -pixelof -width or $widget cget -angstrømof -highlightthickness The places in code are easily identifyable and the addition of -pixelof can almost be automated. dgp added on 2024-09-20 19:36:07: In this example, we are not setting the scrollbar width. We are using [cget] to learn what width Tk has set, and using that to calculate other option values to be consistent. For a script meant to be used on multiple platforms, I think that's the right approach. If I hardcode "11" because it's right on the development system, I probably break the experience for all the users on some other platform. If this is just our imprecise coding practices coming around to bite us, sure, unfortunate. (Still the kind of unfortunate much better found during alpha development, when consequential interface changes like this are less stressfully handled). If it's breakage of common coding idioms found in published example code online or in books, I think that's a bigger deal. No, I can't point to examples. I think the worst part is I have no idea how widespread this problem is and how many of our hundreds of Tk code lines have to be reviewed. Let's at least get a note about the incompatibility into changes.md jan.nijtmans added on 2024-09-20 19:11:25: Workaround: you can set -width/-height/-highlightthickness for the scrollbar to "11", then you will get back exactly what you put in jan.nijtmans added on 2024-09-20 19:07:50: That's - indeed - unfortunate. Still, IMHO, it's the correct behavior not to convert "8.25p" to "11" when retrieving it back. dgp added on 2024-09-20 18:44:55: The change in checkin [24db3e18] breaks our use of Tk. When we use [$scrollbar cget] to retrieve option values such as -width, -height, -highlightthickness, we pass the returned values to [expr] in an expression and this has worked forever. Now there is an error: cannot use non-numeric string "8.25p" as left operand... etc etc Maybe this has always been fragile, and we've always been coding badly (Don't really know), but the fact remains that a post-final-beta change breaks working code. jan.nijtmans added on 2024-09-07 17:42:47: It turns out that the "scrollbar" has exactly the same problem as described here. That part is now fixed [24db3e18b8a9b7b8|here]. Continuing..... chw added on 2024-08-30 05:13:56: > Possible food for another ticket. Sorry for being picky, but for me that is contradicting: * Tk_GetColor() defines an interface (a contract). * Since it is part of the Tk interface, changing it requires a TIP. * But breaking the contract within Tk itself is not a problem. I'm puzzled to say it politely. jan.nijtmans added on 2024-08-29 22:18:02: > I wonder, if all the Tk_Uid contracts to calls like Tk_GetColor() are fulfilled proper. I don't see this Tk_Uids essential property being used in Tk_GetColor(). Various other calls in Tk to Tk_GetColor() don't provide Tk_Uid's. My guess is that the Tk_Uid parameter for Tk_GetColor() should actually just be a "const char *". Possible food for another ticket. Thanks! Now [73d2c175544e39cd|started] the real implementation for this ticket (on top of the changes to DoConfig()). chw added on 2024-08-29 20:13:24: Glimpsing over the changes in the corresponding check-in regarding tclOldConfig.c I wonder, if all the Tk_Uid contracts to calls like Tk_GetColor() are fulfilled proper. Remember the essential property of Tk_Uids: to numerically compare strings by their const'ly represented atomic addresses. In other words, I'd expect more Tk_GetUid() calls all over the tkOldConfig.c module. jan.nijtmans added on 2024-08-28 12:48:25: The problem in the DoConfig() function (in tkOldConfig.c) is that it has a Tk_Uid parameter, not a Tcl_Obj* parameter. So, inside this function, we can call Tcl_GetInt(), not Tcl_GetIntFromObj(). If the parameter is already of "int" type, we still need to parse again the string representation. Solution: Change the parameter in DoConfig() to type Tcl_Obj *. This first part is done [8098b85a16f2addc|here]. Let's see what GITHUB things of that before continuing. jan.nijtmans added on 2024-08-28 07:43:47: > So I'm afraid, changing this will affect the canvas performance I have an idea how to solve this without sacrificing performance. Stay tuned ... jan.nijtmans added on 2024-08-26 09:14:10: I agree that the entry is correct and the canvas is wrong. However, the canvas is still using the old configuration functions, which don't store the value as Tcl_Obj. So I'm afraid, changing this will affect the canvas performance. B.T.W. Tk 8.6 has exactly the same behavior. |