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> <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]" Date: Fri, 7 Oct 2016 16:31:50 -0400 Message-ID: Subject: Re: [geda-user] Segfault bug in PCB? To: geda-user AT delorie DOT com Content-Type: multipart/alternative; boundary=94eb2c18fdb8da4a94053e4c4cdf 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 Precedence: bulk --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

I'm very much still learning t= his 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 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're right that none of the other Erase functions = do this, so you'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't look through enough to see = if that is also be true for any of the other Erase functions.
=C2=A0
=

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.

Or the calls in mo= ve, rotate, etc. are redundant.

=C2=A0

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 applyin= g either solution this was happening to me on both my Mac and my Linux VM. =

=C2=A0

For text objects the Scale parameter is used.=C2=A0 It's type of in= t, but sizeof(Coord) should be >=3D 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 lon= g, than if you cast it as an int, would you get the wrong bytes?

<= div>=C2=A0

But it is very easy to extend the Undo data structure by new "Scal= e" property to safely store the text Scale value. (done in new patch)

Or change Scale to be type C= oord. When I look through global.h I don't see too many things declared= as ints.

--Chad

--94eb2c18fdb8da4a94053e4c4cdf--