Mail Archives: geda-user/2016/10/07/17:49:07
This is a cryptographically signed message in MIME format.
--------------ms030304080706070803020006
Content-Type: multipart/alternative;
boundary="------------F02DB2CED30B267F293F5F81"
This is a multi-part message in MIME format.
--------------F02DB2CED30B267F293F5F81
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: quoted-printable
On 07.10.2016 22:31, Chad Parker (parker DOT charles AT gmail DOT com) [via=20
geda-user AT delorie DOT com] wrote:
> 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=20
> EraseText function altogether? It makes sense to me that a function=20
> with such a name would do both things, but I see that you're right=20
> that none of the other Erase functions do this, so you're probably=20
> right. Either that, or change all of them to match EraseText... :) If=20
> 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=20
> functions removes some duplication of code. You put the call in one=20
> 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.
>
It's not always paired - see ChangeTextJoin function. I did not check=20
other (non-text) functions.
The change of all Erase functions is far beyond scope of bug fix.=20
Anyway, it would introduce some kind of asymmetry for Erase... / Draw... =
function pairs.
>
> 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=20
> doesn't happen. But before applying either solution this was happening =
> to me on both my Mac and my Linux VM.
So we fixed another bug...
> For text objects the Scale parameter is used. It's type of int,
> but sizeof(Coord) should be >=3D than sizeof(int). So for time bein=
g
> 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=20
> you get the wrong bytes?
If you cast it properly, there is no reason to get wrong byte. But the=20
problem was that original Undo code cast the whole structure to Pintype=20
=2E.. it worked for most types, as they share the first part, but Texttyp=
e=20
does not share it, so undo of text size overwrote wrong part of Texttype =
structure (possibly, I did not count bytes).
Both fixes - one with casting, second one with new structure member -=20
solve the issue.
> 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
>
--------------F02DB2CED30B267F293F5F81
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: quoted-printable
<html>
<head>
<meta content=3D"text/html; charset=3Dutf-8" http-equiv=3D"Content-Ty=
pe">
</head>
<body bgcolor=3D"#FFFFFF" text=3D"#000000">
<div class=3D"moz-cite-prefix">On 07.10.2016 22:31, Chad Parker
(<a class=3D"moz-txt-link-abbreviated" href=3D"mailto:parker.charle=
s AT gmail DOT com">parker DOT charles AT gmail DOT com</a>) [via <a class=3D"moz-txt-link-=
abbreviated" href=3D"mailto:geda-user AT delorie DOT com">geda-user AT delorie DOT com<=
/a>] wrote:<br>
</div>
<blockquote
cite=3D"mid:CAJZxidA2JGsuEcyYyf6MC0CAjv=3DFUy-VCW8-PwQXJ22TWRcKFA AT mail DOT gm=
ail.com"
type=3D"cite">
<div dir=3D"ltr"><span class=3D""></span>I'm very much still learni=
ng
this code, so, thanks for your comments.<br>
<div><br>
<div class=3D"gmail_extra">
<div class=3D"gmail_quote">
<blockquote class=3D"gmail_quote" style=3D"margin: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 bgcolor=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 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.<br>
=C2=A0<br>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<br>
It's not always paired - see ChangeTextJoin=C2=A0 function. I did not=
check other (non-text) functions.<br>
<br>
The change of all Erase functions is far beyond scope of bug fix.
Anyway, it would introduce some kind of asymmetry for Erase... /
Draw... function pairs.<br>
<br>
<blockquote
cite=3D"mid:CAJZxidA2JGsuEcyYyf6MC0CAjv=3DFUy-VCW8-PwQXJ22TWRcKFA AT mail DOT gm=
ail.com"
type=3D"cite">
<div dir=3D"ltr">
<div>
<div class=3D"gmail_extra">
<div class=3D"gmail_quote">
<blockquote class=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"><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 IMO redundant.<br>
</div>
</blockquote>
<div><br>
</div>
<div>Or the calls in move, rotate, etc. are redundant.<br>
</div>
<div>=C2=A0<br>
</div>
<blockquote class=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't happen. But before applying
either solution this was happening to me on both my Mac
and my Linux VM. <br>
</div>
<div>=C2=A0</div>
</div>
</div>
</div>
</div>
</blockquote>
<br>
So we fixed another bug...<br>
<br>
<blockquote
cite=3D"mid:CAJZxidA2JGsuEcyYyf6MC0CAjv=3DFUy-VCW8-PwQXJ22TWRcKFA AT mail DOT gm=
ail.com"
type=3D"cite">
<div dir=3D"ltr">
<div>
<div class=3D"gmail_extra">
<div class=3D"gmail_quote">
<blockquote class=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"> For text objec=
ts
the Scale parameter is used.=C2=A0 It's type of int, bu=
t
sizeof(Coord) should be >=3D than sizeof(int). So fo=
r
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 long, than if you cast it as
an int, would you get the wrong bytes?<br>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<br>
If you cast it properly, there is no reason to get wrong byte.=C2=A0 =
But
the problem was that original Undo code cast the whole structure to
Pintype ... it worked for most types, as they share the first part,
but Texttype does not share it, so undo of text size overwrote wrong
part of Texttype structure (possibly, I did not count bytes).<br>
<br>
Both fixes - one with casting, second one with new structure member
- solve the issue.<br>
<br>
<blockquote
cite=3D"mid:CAJZxidA2JGsuEcyYyf6MC0CAjv=3DFUy-VCW8-PwQXJ22TWRcKFA AT mail DOT gm=
ail.com"
type=3D"cite">
<div dir=3D"ltr">
<div>
<div class=3D"gmail_extra">
<div class=3D"gmail_quote">
<div>=C2=A0</div>
<blockquote class=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"> 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)<br>
<br>
</div>
</blockquote>
<div><br>
</div>
<div>Or change Scale to be type Coord. When I look through
global.h I don't see too many things declared as ints. <b=
r>
</div>
<div><br>
</div>
<div>--Chad<br>
</div>
</div>
<br>
</div>
</div>
</div>
</blockquote>
<p><br>
</p>
</body>
</html>
--------------F02DB2CED30B267F293F5F81--
--------------ms030304080706070803020006
Content-Type: application/pkcs7-signature; name="smime.p7s"
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="smime.p7s"
Content-Description: S/MIME Cryptographic Signature
MIAGCSqGSIb3DQEHAqCAMIACAQExDzANBglghkgBZQMEAgEFADCABgkqhkiG9w0BBwEAAKCC
CtowggTwMIID2KADAgECAhBqqLAGfunD4XYlcEsP4ElzMA0GCSqGSIb3DQEBCwUAMHUxCzAJ
BgNVBAYTAklMMRYwFAYDVQQKEw1TdGFydENvbSBMdGQuMSkwJwYDVQQLEyBTdGFydENvbSBD
ZXJ0aWZpY2F0aW9uIEF1dGhvcml0eTEjMCEGA1UEAxMaU3RhcnRDb20gQ2xhc3MgMSBDbGll
bnQgQ0EwHhcNMTYwMzA3MDEwODU2WhcNMTcwMzA3MDEwODU2WjA8MRkwFwYDVQQDDBBtaWxh
bkBwcm9jaGFjLnNrMR8wHQYJKoZIhvcNAQkBFhBtaWxhbkBwcm9jaGFjLnNrMIIBIjANBgkq
hkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0mKGFeH4hukzo8EbR08H7ZFZ1uAI3mTA51yN6C/Q
R++4fhe2R6U1yGR2bQeWal47aLFt4bhUEED1cvU4GDT+tzJUeQMSghB1qwrPQfzJueT3ReYA
5uaym6q2tFR9qm5lhSWqac//+mPFThodIz7rMT4a5R2ZnQjBSiEOVP73Sf0jiD7ylNBqvkCd
hKFTSt7IkoRCkVE8o7KYEjzEhsSu1JAuCh8rYpz1vajpvO+H3FJyDXbHqCxH4MTmA1PMkObc
BPHnsA4NE9xJpghiqJWUyEmcrNSRikLgJY2o0bO2y7AZ8AfXGt1lCkcg2f879JxuswDYaQFH
rfPERgaIJ2OCLQIDAQABo4IBszCCAa8wDgYDVR0PAQH/BAQDAgSwMB0GA1UdJQQWMBQGCCsG
AQUFBwMCBggrBgEFBQcDBDAJBgNVHRMEAjAAMB0GA1UdDgQWBBQBZgVnt+lqmMtzm6Wfvlmr
aFGERzAfBgNVHSMEGDAWgBQkgWw5Yb5JD4+3G0YrySi1J0htaDBvBggrBgEFBQcBAQRjMGEw
JAYIKwYBBQUHMAGGGGh0dHA6Ly9vY3NwLnN0YXJ0c3NsLmNvbTA5BggrBgEFBQcwAoYtaHR0
cDovL2FpYS5zdGFydHNzbC5jb20vY2VydHMvc2NhLmNsaWVudDEuY3J0MDgGA1UdHwQxMC8w
LaAroCmGJ2h0dHA6Ly9jcmwuc3RhcnRzc2wuY29tL3NjYS1jbGllbnQxLmNybDAbBgNVHREE
FDASgRBtaWxhbkBwcm9jaGFjLnNrMCMGA1UdEgQcMBqGGGh0dHA6Ly93d3cuc3RhcnRzc2wu
Y29tLzBGBgNVHSAEPzA9MDsGCysGAQQBgbU3AQIEMCwwKgYIKwYBBQUHAgEWHmh0dHA6Ly93
d3cuc3RhcnRzc2wuY29tL3BvbGljeTANBgkqhkiG9w0BAQsFAAOCAQEAcdXDR99mAwWbAilc
x4iAlxzxTpAFBwXjDbFUFuLXHtXUQjfQb6isV/A6HGhFfEeloJ/dl0hZNfn6ROken+Pv8UzA
sgbpjVQHVx334qK38Fbsoy/L991YYRyJGGl7OJYRs7SY8gJ+kVeC4pN6tdeACvmxdRYyA7r0
AjwhzCOpc449UQO2YQRZ6uzXvo3auQhkFQDCN6Gzn82fr5KRQ26fzUYOphHjkNTUHmYJRbjA
QPIJlYSoc9/Gayl3tHscOt8pSXsfJ2alDnD5Ww94E7corjF7kkPdHQvkvZY5Ux72B7eEbSm8
w+FnngjA2Jc7o25oU+weiVOeBGxjo+HlwjaqPzCCBeIwggPKoAMCAQICEGunin0K14jWUQr5
WeTntOEwDQYJKoZIhvcNAQELBQAwfTELMAkGA1UEBhMCSUwxFjAUBgNVBAoTDVN0YXJ0Q29t
IEx0ZC4xKzApBgNVBAsTIlNlY3VyZSBEaWdpdGFsIENlcnRpZmljYXRlIFNpZ25pbmcxKTAn
BgNVBAMTIFN0YXJ0Q29tIENlcnRpZmljYXRpb24gQXV0aG9yaXR5MB4XDTE1MTIxNjAxMDAw
NVoXDTMwMTIxNjAxMDAwNVowdTELMAkGA1UEBhMCSUwxFjAUBgNVBAoTDVN0YXJ0Q29tIEx0
ZC4xKTAnBgNVBAsTIFN0YXJ0Q29tIENlcnRpZmljYXRpb24gQXV0aG9yaXR5MSMwIQYDVQQD
ExpTdGFydENvbSBDbGFzcyAxIENsaWVudCBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCC
AQoCggEBAL192vfDon2D9luC/dtbX64eG3XAtRmvmCSsu1d52DXsCR58zJQbCtB2/A5uFqNx
WacpXGGtTCRk9dEDBlmixEd8QiLkUfvHpJX/xKnmVkS6Iye8wUbYzMsDzgnpazlPg19dnSqf
hM+Cevdfa89VLnUztRr2cgmCfyO9Otrh7LJDPG+4D8ZnAqDtVB8MKYJL6QgKyVhhaBc4y3bG
WxKyXEtx7QIZZGxPwSkzK3WIN+VKNdkiwTubW5PIdopmykwvIjLPqbJK7yPwFZYekKE015Os
W6FV+s4DIM8UlVS8pkIsoGGJtMuWjLL4tq2hYQuuN0jhrxK1ljz50hH23gA9cbMCAwEAAaOC
AWQwggFgMA4GA1UdDwEB/wQEAwIBBjAdBgNVHSUEFjAUBggrBgEFBQcDAgYIKwYBBQUHAwQw
EgYDVR0TAQH/BAgwBgEB/wIBADAyBgNVHR8EKzApMCegJaAjhiFodHRwOi8vY3JsLnN0YXJ0
c3NsLmNvbS9zZnNjYS5jcmwwZgYIKwYBBQUHAQEEWjBYMCQGCCsGAQUFBzABhhhodHRwOi8v
b2NzcC5zdGFydHNzbC5jb20wMAYIKwYBBQUHMAKGJGh0dHA6Ly9haWEuc3RhcnRzc2wuY29t
L2NlcnRzL2NhLmNydDAdBgNVHQ4EFgQUJIFsOWG+SQ+PtxtGK8kotSdIbWgwHwYDVR0jBBgw
FoAUTgvvGqRAW6UXaYcwyjRoQ9BBrvIwPwYDVR0gBDgwNjA0BgRVHSAAMCwwKgYIKwYBBQUH
AgEWHmh0dHA6Ly93d3cuc3RhcnRzc2wuY29tL3BvbGljeTANBgkqhkiG9w0BAQsFAAOCAgEA
i+P3h+wBi4StDwECW5zhIycjBL008HACblIf26HY0JdOruKbrWDsXUsiI0j/7Crft9S5oxvP
iDtVqspBOB/y5uzSns1lZwh7sG96bYBZpcGzGxpFNjDmQbcM3yl3WFIRS4WhNrsOY14V7y2I
rUGsvetsD+bjyOngCIVeC/GmsmtbuLOzJ606tEc9uRbhjTu/b0x2Fo+/e7UkQvKzNeo7OMhi
jixaULyINBfCBJb+e29bLafgu6JqjOUJ9eXXj20p6q/CW+uVrZiSW57+q5an2P2i7hP85jQJ
cy5j4HzA0rSiF3YPhKGAWUxKPMAVGgcYoXzWydOvZ3UDsTDTagXpRDIKQLZo02wrlxY6iMFq
vlzsemVf1odhQJmi7Eh5TbxI40kDGcBOBHhwnaOumZhLP+SWJQnjpLpSlUOj95uf1zo9oz9e
0NgIJoz/tdfrBzez76xtDsK0KfUDHt1/q59BvDI7RX6gVr0fQoCyMczNzCTcRXYHY0tq2J0o
T+bsb6sH2b4WVWAiJKnSYaWDjdA70qHX4mq9MIjO/ZskmSY8wtAk24orAc0vwXgYanqNsBX5
Yv4sN4Z9VyrwMdLcusP7HJgRdAGKpkR2I9U4zEsNJQJewM7S4Jalo1DyPrLpL2nTET8ZrSl5
Utp1UeGp/2deoprGevfnxWB+vHNQiu85o6MxggPMMIIDyAIBATCBiTB1MQswCQYDVQQGEwJJ
TDEWMBQGA1UEChMNU3RhcnRDb20gTHRkLjEpMCcGA1UECxMgU3RhcnRDb20gQ2VydGlmaWNh
dGlvbiBBdXRob3JpdHkxIzAhBgNVBAMTGlN0YXJ0Q29tIENsYXNzIDEgQ2xpZW50IENBAhBq
qLAGfunD4XYlcEsP4ElzMA0GCWCGSAFlAwQCAQUAoIICEzAYBgkqhkiG9w0BCQMxCwYJKoZI
hvcNAQcBMBwGCSqGSIb3DQEJBTEPFw0xNjEwMDcyMTQ2MDVaMC8GCSqGSIb3DQEJBDEiBCCg
fw831hxOOllpDcIGljGehYhzaDql3F28EAKqOqKDozBsBgkqhkiG9w0BCQ8xXzBdMAsGCWCG
SAFlAwQBKjALBglghkgBZQMEAQIwCgYIKoZIhvcNAwcwDgYIKoZIhvcNAwICAgCAMA0GCCqG
SIb3DQMCAgFAMAcGBSsOAwIHMA0GCCqGSIb3DQMCAgEoMIGaBgkrBgEEAYI3EAQxgYwwgYkw
dTELMAkGA1UEBhMCSUwxFjAUBgNVBAoTDVN0YXJ0Q29tIEx0ZC4xKTAnBgNVBAsTIFN0YXJ0
Q29tIENlcnRpZmljYXRpb24gQXV0aG9yaXR5MSMwIQYDVQQDExpTdGFydENvbSBDbGFzcyAx
IENsaWVudCBDQQIQaqiwBn7pw+F2JXBLD+BJczCBnAYLKoZIhvcNAQkQAgsxgYyggYkwdTEL
MAkGA1UEBhMCSUwxFjAUBgNVBAoTDVN0YXJ0Q29tIEx0ZC4xKTAnBgNVBAsTIFN0YXJ0Q29t
IENlcnRpZmljYXRpb24gQXV0aG9yaXR5MSMwIQYDVQQDExpTdGFydENvbSBDbGFzcyAxIENs
aWVudCBDQQIQaqiwBn7pw+F2JXBLD+BJczANBgkqhkiG9w0BAQEFAASCAQCObZDyJTtIVB65
ppKbp9aqzKEIGsRCKZMvlifpO14Ja2OdvK+pLvSpmr6t+EKh02AhLusPlbhgUhRtkL4op+VA
KOY+BxhAvVDiFmoMWaULydUpPWDIK6+v2TeM6ddZZ9S1f1aFEr8mxK3CFCKv4YQaNzZ0kTov
nUGSWkzebwaodocKDFrUw17xa05qXadCHffOkQS5enfuAm2CDl1o7aA80VEl5upUNIZjhQL7
anGMDGETD0pAmN9RhmmgmEpYwLhVswVd3byuSrO2247xZ9s39BJNTnFvJG1OmNqIzyxF8qls
Nq/qf8McHpH86Q62h6Y/j/nkHJeZHy1hYcqPXqYXAAAAAAAA
--------------ms030304080706070803020006--
- Raw text -