Tk Source Code

View Ticket
Login
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.