X-Authentication-Warning:	delorie.com: mail set sender to geda-user-bounces using -f
X-Recipient:	geda-user AT delorie DOT com
X-Original-DKIM-Signature:	v=1; a=rsa-sha256; c=relaxed/relaxed;
	d=gmail.com; s=20120113;
	h=mime-version:in-reply-to:references:from:date:message-id:subject:to;
	bh=47fw/0/q2yeO21z1HxuFvSPi3he0Btf8gS5ZWy8i6m4=;
	b=CfypqA6nr2KVlLvunRimetyJe/EwBpeI9wilULS0RvEW479lG7Cym2EM8hnlg1Sc3K
	8/jrdXz9sjlrwHhagFMtz5p9fvEa1JNk6U2jSu2dxY00LnuQwMk1HSFpI9Y0VADwiuiS
	FmiIltVQsQEdvuW59KZLPUGn/N3a3SfKuTwg68P5qSmQnkXCzPA4qZzihiUUmOaWrTzg
	q7HOWL/fEDodyMZNro8ezTrmptpOOnZwiJfRNSIrZ/KoxpnyikFE/JAqQU4jUsACngZh
	51u28muehPxulv+KFs99Rwi3eyvJRztPsYQJikjXC/HPrd8QpPp4gkpHNC/O0hUZpwp+
	FBNg==
X-Google-DKIM-Signature:	v=1; a=rsa-sha256; c=relaxed/relaxed;
	d=1e100.net; s=20130820;
	h=x-gm-message-state:mime-version:in-reply-to:references:from:date
	:message-id:subject:to;
	bh=47fw/0/q2yeO21z1HxuFvSPi3he0Btf8gS5ZWy8i6m4=;
	b=PcwaMTgf1qT3Nx8mNoAlCEP6M8BbqfDySnOc24BhiodoEoTXrYs/hlPQr1PXS+5F7Q
	VE3A7HXfMWjIcjDtgJkNfepFU01uHVBJuWdWlnmfDEjtlqwjHsw628xjrBkuVsS47g7T
	jQYL2JjEYqGEPetzwuy+3p1WRym328Ub+Jm7hW4W/LZozDxSGjSFF2EDBST1v4ljy7z+
	hlLnOi0sR8iCIqaTnD0PqeiGHYAwk+qdwwlKfe+S0w7SgHaZZTI8VNf3gcBReZ6RxFTp
	+0qzBvCQ8qce1Ti90kRrw0pPCuZXtsYZRu9WjfmRFZmSAdaNYGPAc88eN/UfklihWvOh
	QCeg==
X-Gm-Message-State:	AA6/9Rm8gJ0SarWfB4QDI9hr2op/LQog+NzsnANHPl5tbRP5nj38z5GjunDpzUlwZVJRPky5EckO2Jvg6+S7zQ==
X-Received:	by 10.176.0.180 with SMTP id 49mr12743300uaj.174.1475851281271;
	Fri, 07 Oct 2016 07:41:21 -0700 (PDT)
MIME-Version:	1.0
In-Reply-To:	<dc361824-3efa-20c7-dc0e-2dfb3530381b@prochac.sk>
References:	<1475747077 DOT 18232 DOT 10 DOT camel AT linetec> <dc361824-3efa-20c7-dc0e-2dfb3530381b AT prochac DOT sk>
From:	"Chad Parker (parker DOT charles AT gmail DOT com) [via geda-user AT delorie DOT com]" <geda-user AT delorie DOT com>
Date:	Fri, 7 Oct 2016 10:41:20 -0400
Message-ID:	<CAJZxidDmtWyBe+HSMG3ceAejG3czdMy24G+N+RGW+zNBKNLpJg@mail.gmail.com>
Subject:	Re: [geda-user] Segfault bug in PCB?
To:	geda-user AT delorie DOT com
Reply-To:	geda-user AT delorie DOT com
Errors-To:	nobody AT delorie DOT com
X-Mailing-List:	geda-user AT delorie DOT com
X-Unsubscribes-To:	listserv AT delorie DOT com

--001a113dc4165c92e5053e47674f
Content-Type: text/plain; charset=UTF-8

Someone with more experience please correct me if I'm wrong, but I think
that removing the text from the rtree is basically the entire point of
EraseText. It removes the object from the rtree, and flags the area for
being redrawn. If the object is removed from the rtree then it wont be
found when drawing occurs, and so it wont be drawn. So, I'm not sure that
commenting that line is the right thing to do.

It looks like, in most cases, you can get away with commenting this line
because almost all of the functions that call EraseText also try to remove
the object from the rtree anyway, but not all of them. If you compile with
debugging enabled, you get a failed assertion in when you change text. Just
put some text on an empty layout, put the arrow over it, hit n, input new
text, hit enter, and crash.

I think maybe a the error is actually in EraseObject, which calls EraseText
for both TEXT_TYPE and ELEMENTNAME_TYPE. This is problematic for two
reasons. First, if EraseObject is getting an element name, then as you
noted, lptr will be pointing to an element not a layer (the result of
SearchObjectByID), as you pointed out. Second, the text types associated
with element names are not stored in the layer rtree, but rather in one of
the PCB->Data rtrees.

I think a better solution is that when EraseObject gets an
ELEMENTNAME_TYPE, it should call EraseElementName instead of EraseText.

I pushed this fix to the repo home/cparker/LP1631059.

This has highlighted a couple other problems though. When you undo changing
the size of a text type (not an element name) it disappears. There's also
an issue of UndoChangeSize using the Thickness parameter, which is of type
Coord, but for text objects, the Size parameter is of type int. So there's
still a mismatch there. Could be the cause and effect. I'll look into it a
bit more later.

--Chad



On Thu, Oct 6, 2016 at 12:26 PM, Milan Prochac (milan AT prochac DOT sk) [via
geda-user AT delorie DOT com] <geda-user AT delorie DOT com> wrote:

> On 06.10.2016 11:44, Richard Rasker (rasker AT linetec DOT nl) [via
> geda-user AT delorie DOT com] wrote:
>
>> Hello,
>>
>> There appears to be a little segfault problem in PCB version 1.99z,
>> build Sep 13 2016 at 08:12:51, used on Linux Mint 17.3 KDE:
>>
>> When I change the size of a RefDes text using the S key, and then try to
>> undo this action using the U key, PCB crashes. This also happens when
>> the resize action is followed by other actions; when subsequently
>> undoing these using U, PCB crashes the moment that it tries to undo the
>> RefDes text resize. As far as I can tell, this bug only pertains to
>> Refdes text, not to other elements.
>>
>> It's not a big deal, but still a little annoying to have some minutes of
>> work vanish when unthinkingly pressing the U key.
>>
>> Best regards,
>>
>> Richard Rasker
>>
>
> Bug logged and patch provided here: https://bugs.launchpad.net/pcb
> /+bug/1631059
>
> Patch solves the issue (2 issues in fact), undo/redo works fine. But
> second pair of eyes should check if the patch has some unwanted side
> effects. More details on Launchpad.
>
> Milan
>
>
>

--001a113dc4165c92e5053e47674f
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>Someone with more experience please correct me if I&#=
39;m wrong, but I think that removing the text from the rtree is basically =
the entire point of EraseText. It removes the object from the rtree, and fl=
ags the area for being redrawn. If the object is removed from the rtree the=
n it wont be found when drawing occurs, and so it wont be drawn. So, I&#39;=
m not sure that commenting that line is the right thing to do.<br><br></div=
><div>It looks like, in most cases, you can get away with commenting this l=
ine because almost all of the functions that call EraseText also try to rem=
ove the object from the rtree anyway, but not all of them. If you compile w=
ith debugging enabled, you get a failed assertion in when you change text. =
Just put some text on an empty layout, put the arrow over it, hit n, input =
new text, hit enter, and crash.<br><br></div><div>I think maybe a the error=
 is actually in EraseObject, which calls EraseText for both TEXT_TYPE and E=
LEMENTNAME_TYPE. This is problematic for two reasons. First, if EraseObject=
 is getting an element name, then as you noted, lptr will be pointing to an=
 element not a layer (the result of SearchObjectByID), as you pointed out. =
Second, the text types associated with element names are not stored in the =
layer rtree, but rather in one of the PCB-&gt;Data rtrees.<br><br></div><di=
v>I think a better solution is that when EraseObject gets an ELEMENTNAME_TY=
PE, it should call EraseElementName instead of EraseText.<br><br></div><div=
>I pushed this fix to the repo home/cparker/LP1631059.<br><br></div><div>Th=
is has highlighted a couple other problems though. When you undo changing t=
he size of a text type (not an element name) it disappears. There&#39;s als=
o an issue of UndoChangeSize using the Thickness parameter, which is of typ=
e Coord, but for text objects, the Size parameter is of type int. So there&=
#39;s still a mismatch there. Could be the cause and effect. I&#39;ll look =
into it a bit more later.<br><br></div><div>--Chad<br></div><div><br></div>=
<div><br></div></div><div class=3D"gmail_extra"><br><div class=3D"gmail_quo=
te">On Thu, Oct 6, 2016 at 12:26 PM, Milan Prochac (<a href=3D"mailto:milan=
@prochac.sk">milan AT prochac DOT sk</a>) [via <a href=3D"mailto:geda-user AT delorie=
.com">geda-user AT delorie DOT com</a>] <span dir=3D"ltr">&lt;<a href=3D"mailto:ge=
da-user AT delorie DOT com" target=3D"_blank">geda-user AT delorie DOT com</a>&gt;</span>=
 wrote:<br><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;bor=
der-left:1px #ccc solid;padding-left:1ex"><div class=3D"HOEnZb"><div class=
=3D"h5">On 06.10.2016 11:44, Richard Rasker (<a href=3D"mailto:rasker AT linet=
ec.nl" target=3D"_blank">rasker AT linetec DOT nl</a>) [via <a href=3D"mailto:geda=
-user AT delorie DOT com" target=3D"_blank">geda-user AT delorie DOT com</a>] wrote:<br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex">
Hello,<br>
<br>
There appears to be a little segfault problem in PCB version 1.99z,<br>
build Sep 13 2016 at 08:12:51, used on Linux Mint 17.3 KDE:<br>
<br>
When I change the size of a RefDes text using the S key, and then try to<br=
>
undo this action using the U key, PCB crashes. This also happens when<br>
the resize action is followed by other actions; when subsequently<br>
undoing these using U, PCB crashes the moment that it tries to undo the<br>
RefDes text resize. As far as I can tell, this bug only pertains to<br>
Refdes text, not to other elements.<br>
<br>
It&#39;s not a big deal, but still a little annoying to have some minutes o=
f<br>
work vanish when unthinkingly pressing the U key.<br>
<br>
Best regards,<br>
<br>
Richard Rasker<br>
</blockquote>
<br></div></div>
Bug logged and patch provided here: <a href=3D"https://bugs.launchpad.net/p=
cb/+bug/1631059" rel=3D"noreferrer" target=3D"_blank">https://bugs.launchpa=
d.net/pcb<wbr>/+bug/1631059</a><br>
<br>
Patch solves the issue (2 issues in fact), undo/redo works fine. But second=
 pair of eyes should check if the patch has some unwanted side effects. Mor=
e details on Launchpad.<br>
<br>
Milan<br>
<br>
<br>
</blockquote></div><br></div>

--001a113dc4165c92e5053e47674f--

- Raw text -