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=IkhyxcMby8ipshcc3fVF9adfrWjGmrtIiT77Xcc5duM=;
	b=yfROPASHrvgpkhLTfIOsfyN/7ekrdZ0A/GM899whWlQoYATY8sxYDvm9r8nGyqQOba
	td7rRCRQmZ8n0xYtx2XqwYwwAAi9D6WjZrXzfM19QR0++PlcY4hzIAUX0hBoNYLuz5eM
	UqN19YvOhs/lgEPwhMnSoqfFb/qveScUHNJQHJP0BnDqf4+NQwgnewDzB4/qKHjySPss
	Gngs+/DqOTKXOYc/g6IBWYHackERqpV9HZCFW2AxY3gxgDTfzMgV/ZAl6LGN52QpEMmT
	T2JU8bskmAEGhv4/OIMWJSOsaQtBH6FL+pzu2spKdJMOrZjM9npDDIHTBLuuSfQovbS7
	tt/Q==
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=IkhyxcMby8ipshcc3fVF9adfrWjGmrtIiT77Xcc5duM=;
	b=PRHfkFDN+o8rOGsieU3BCn92Qfw64AVa4DfFUl//YZ7rDCw+Wpeh43iwv4HvEGSUEo
	RoA8ch9q43Iz2QxEM+AuQNOq/AGkN+dteGzVh4dW921z+AZEVKpA/53Jqg0qZhFpwyuV
	V52J/cJJ/j5ckgSm6MJaX2JDPFf988T+cMAAq8Fh1pYdfHsIcaR5yNWeb32vA7JYBd4B
	bNPG8dlbwrARoDIbdnQSPUZ4K9I8vYXdAd4+56FOteMsykuPHfoYhpoJTXMPpK7eVUVW
	cOlwFGt6FspAdDvvqBxBXYlaIYYJ4CTsjgi8B7qpXOFuUPfiWHb71NMclpYJfE86RNPD
	D2Ig==
X-Gm-Message-State:	AA6/9Rk6X8lFi+pZeS3riLfoWkGS5H8apWJPvFBESzaMOGPsdJ2TEd3PSieuibgW6rg3HbZIP9mEo0WbijuTAw==
X-Received:	by 10.176.82.122 with SMTP id j55mr8930434uaa.109.1475872311362;
	Fri, 07 Oct 2016 13:31:51 -0700 (PDT)
MIME-Version:	1.0
In-Reply-To:	<5d74ff3f-1470-7745-33b3-385d98cfb961@prochac.sk>
References:	<1475747077 DOT 18232 DOT 10 DOT camel AT linetec> <dc361824-3efa-20c7-dc0e-2dfb3530381b AT prochac DOT sk>
	<CAJZxidDmtWyBe+HSMG3ceAejG3czdMy24G+N+RGW+zNBKNLpJg AT mail DOT gmail DOT com> <5d74ff3f-1470-7745-33b3-385d98cfb961 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 16:31:50 -0400
Message-ID:	<CAJZxidA2JGsuEcyYyf6MC0CAjv=FUy-VCW8-PwQXJ22TWRcKFA@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

--94eb2c18fdb8da4a94053e4c4cdf
Content-Type: text/plain; charset=UTF-8

I'm very much still learning this code, so, thanks for your comments.


> EraseText also adds the bounding box of text to area to be redrawn, like
> any other Erase... functions. But only EraseText removes the entity from
> rtree, no other Erase... function does it. I think that major purpose of
> EraseText function should be to invalidate drawing area. Just for code
> clarity.
>
>
Then why not just invalidate the drawing area directly and skip the
EraseText function altogether? It makes sense to me that a function with
such a name would do both things, but I see that you're right that none of
the other Erase functions do this, so you're probably right. Either that,
or change all of them to match EraseText... :) If the Erase functions are
always paired with a r_delete_entry (as is the case with EraseText), then
putting the r_delete_entry in the Erase functions removes some duplication
of code. You put the call in one place instead of a dozen. I haven't look
through enough to see if that is also be true for any of the other Erase
functions.


>
> But other functions - like text move, rotate - use r_delete_entry and
> r_insert_entry in pairs and r_delete_entry in EraseText is IMO redundant.
>

Or the calls in move, rotate, etc. are redundant.


>
> I did not notice that any text disappearing while undoing size change...
> but it disappears after undoing the text creation :)
>
>
If you delete the call to r_delete_entry in EraseText, then this doesn't
happen. But before applying either solution this was happening to me on
both my Mac and my Linux VM.


> For text objects the Scale parameter is used.  It's type of int, but
> sizeof(Coord) should be >= than sizeof(int). So for time being it should be
> OK to accept the type impurity without changing the data structures.
>
>
If Coord translates to a long, than if you cast it as an int, would you get
the wrong bytes?


> But it is very easy to extend the Undo data structure by new "Scale"
> property to safely store the text Scale value. (done in new patch)
>
>
Or change Scale to be type Coord. When I look through global.h I don't see
too many things declared as ints.

--Chad

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

<div dir=3D"ltr"><span class=3D""></span>I&#39;m very much still learning t=
his code, so, thanks for your comments.<br><div><br><div class=3D"gmail_ext=
ra"><div class=3D"gmail_quote"><blockquote class=3D"gmail_quote" style=3D"m=
argin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=
=3D""></span><span class=3D""></span><br><span class=3D""></span><div bgcol=
or=3D"#FFFFFF" text=3D"#000000">
    EraseText also adds the bounding box of text to area to be redrawn,
    like any other Erase... functions. But only EraseText removes the
    entity from rtree, no other Erase... function does it. I think that
    major purpose of EraseText function should be to invalidate drawing
    area. Just for code clarity.<span class=3D""><br>
    <br></span></div></blockquote><div><br></div><div>Then why not just inv=
alidate the drawing area directly and skip the EraseText function altogethe=
r? It makes sense to me that a function with such a name would do both thin=
gs, but I see that you&#39;re right that none of the other Erase functions =
do this, so you&#39;re probably right. Either that, or change all of them t=
o match EraseText... :) If the Erase functions are always paired with a r_d=
elete_entry (as is the case with EraseText), then putting the r_delete_entr=
y in the Erase functions removes some duplication of code. You put the call=
 in one place instead of a dozen. I haven&#39;t look through enough to see =
if that is also be true for any of the other Erase functions.<br>=C2=A0<br>=
</div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-l=
eft:1px #ccc solid;padding-left:1ex"><div bgcolor=3D"#FFFFFF" text=3D"#0000=
00"><br>
    But other functions - like text move, rotate - use r_delete_entry
    and=C2=A0 r_insert_entry in pairs and r_delete_entry in EraseText is IM=
O
    redundant.<br></div></blockquote><div><br></div><div>Or the calls in mo=
ve, rotate, etc. are redundant.<br></div><div>=C2=A0<br></div><blockquote c=
lass=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;=
padding-left:1ex"><div bgcolor=3D"#FFFFFF" text=3D"#000000">
    <span class=3D""><br></span>
    I did not notice that any text disappearing while undoing size
    change... but it disappears after undoing the text creation :)<br>
    <br></div></blockquote><div><br></div><div>If you delete the call to r_=
delete_entry in EraseText, then this doesn&#39;t happen. But before applyin=
g either solution this was happening to me on both my Mac and my Linux VM. =
<br></div><div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margi=
n:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor=3D"#=
FFFFFF" text=3D"#000000">
    For text objects the Scale parameter is used.=C2=A0 It&#39;s type of in=
t, but
    sizeof(Coord) should be &gt;=3D than sizeof(int). So for time being it
    should be OK to accept the type impurity without changing the data
    structures. <br>
    <br></div></blockquote><div><br></div><div>If Coord translates to a lon=
g, than if you cast it as an int, would you get the wrong bytes?<br></div><=
div>=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8e=
x;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor=3D"#FFFFFF" tex=
t=3D"#000000">
    But it is very easy to extend the Undo data structure by new &quot;Scal=
e&quot;
    property to safely store the text Scale value. (done in new patch)<br>
    <br></div></blockquote><div><br></div><div>Or change Scale to be type C=
oord. When I look through global.h I don&#39;t see too many things declared=
 as ints. <br></div><div><br></div><div>--Chad<br></div></div><br></div></d=
iv></div>

--94eb2c18fdb8da4a94053e4c4cdf--

- Raw text -