#927 canvas dash update problem

[Brent B. Welch]

OriginalBugID: 5997 Bug
Version: 8.4a1
SubmitDate: '2000-07-10'
LastModified:
Severity: MED
Status: UnAssn
Submitter: techsupp
OS: Linux-Red Hat
OSVersion: see below.
Machine: Linux gitarre 2.2.15 #1 Wed May 10 22:58:59 CEST 2000 i686 unkno

Name:
Matthias Jung

Extensions:
none

CustomShell:
no.

Comments:
If other dash pattern styles are specified, e.g. {40 1}, the line IS
updated. E.g. the above
error can be worked around by specifying

.c itemconfigure 1 -dash {40 1}
.c itemconfigure 1 -dash {40 40}

ReproducibleScript:
canvas .c
pack .c
.c create line 1 1 300 200 -dash {15 15}
.c itemconfigure 1 -dash {40 40}

ObservedBehavior:
Line is not updated to the new dash pattern.

DesiredBehavior:
Updated dash pattern.

[Sebastian Wangnick]

Logged In: YES
user_id=202812
Originator: NO

This is because ANY dash setting of the form {n n} is disregarded, and rather dash 4 is implemented. See tkCanvUtil.c:
* if (dash->number >= 2) {
* gcValues->dashes = 4;
* } else
if (dash->number > 0) {
gcValues->dashes = dash->pattern.array[0];
} else {
gcValues->dashes = (char) (4 * width);
}

and then:
} else if ( dash->number>2 || (dash->number==2 &&
(dash->pattern.array[0]!=dash->pattern.array[1]))) {
p = (char *) (dash->number > sizeof(char *)) ? dash->pattern.pt : dash->pattern.array;
XSetDashes(((TkCanvas *)canvas)->display, outline->gc, outline->offset, p, dash->number);
}

To fix this problem the lines marked * above must be deleted.

[Sebastian Wangnick]

Logged In: YES
user_id=202812
Originator: NO

Note that there is another bug hidden in the canvas dash handling routines:

* if ((dash->number<-1) || ((dash->number == -1) && (dash->pattern.array[1]!=','))) {
char *q;
int i = -dash->number;

p = (i > sizeof(char *)) ? dash->pattern.pt : dash->pattern.array;
q = (char *) ckalloc(2*(unsigned int)i);
i = DashConvert(q, p, i, width);
XSetDashes(((TkCanvas *)canvas)->display, outline->gc, outline->offset, q, i);
ckfree(q);
} else ...

In the line * above, it should be dash->pattern.array[0]!=','

To provoke the error, create a line item with dash, say, ",,", and then reconfigure the dash to ".", and note that this will erroneously not change the pattern (since the second char in the pattern array is still ",").

[Sebastian Wangnick]

Logged In: YES
user_id=202812
Originator: NO

Note that the two patches described earlier must also be reflected in Tk_ResetOutlineGC.

[Jeffrey Hobbs]

Logged In: YES
user_id=72656
Originator: NO

Could you please provide a clear patch (diff -u format) for those items for testing?

[Andy Goth]

I also ran into the comma problem. I am attaching a patch that fixes it.

[Andy Goth]

Never mind. It won't let me attach. So I will paste the patch into this comment.

diff -ur tk8.6b1~/generic/tkCanvUtil.c tk8.6b1/generic/tkCanvUtil.c
--- tk8.6b1~/generic/tkCanvUtil.c 2008-11-27 17:47:09.000000000 -0600
+++ tk8.6b1/generic/tkCanvUtil.c 2009-06-20 12:06:42.000000000 -0500
@@ -1207,7 +1207,7 @@
}

if ((dash->number<-1) ||
- ((dash->number == -1) && (dash->pattern.array[1] != ','))) {
+ ((dash->number == -1) && (dash->pattern.array[0] != ','))) {
char *q;
int i = -dash->number;

@@ -1327,7 +1327,7 @@

if ((dash->number > 2) || (dash->number < -1) || (dash->number==2 &&
(dash->pattern.array[0] != dash->pattern.array[1])) ||
- ((dash->number == -1) && (dash->pattern.array[1] != ','))) {
+ ((dash->number == -1) && (dash->pattern.array[0] != ','))) {
if (dash->number < 0) {
dashList = (int) (4 * width + 0.5);
} else if (dash->number<3) {

[Andy Goth]

Maybe it might help if I explain the rationale for this test. The default X11 dash pattern is identical to the "," dash pattern (I checked), so this test tries to avoid a redundant invocation of XSetDashes. In English: "if string length is 1 and first character is comma, then don't call XSetDashes". (The logic is inverted in the actual implementation.) But the implementation inadvertently uses dash->pattern.array[1] to read the first character, which is of course the second character.

[Jan Nijtmans]

fixed in tkCanvUtil.c 1.25. Issue left open for backport. Thanks, Andy!

[Jan Nijtmans]

Sabastian was right about the two lines that should be deleted,
checked in this change now. But that's not the complete story.
The two functions Tk_ConfigOutlineGC() and Tk_ResetOutlineGC()
should always leave the GC in the same state, and the "if"
statements there are not exactly the same. Fixed that too.

Now I know why I didn't notice this bug before: In stead of
"dash {40 40}" it's equivalent to simply do "dash 40" and
that's what I always use. This bug only occured with two-value
dash lists in which the two values were identical.

Finally, after more than 10 years, it's fixed now.

Backported to 8.5 as well, so now can be closed.