<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 12, 2014 at 5:38 PM, Dirk Hohndel <span dir="ltr"><<a href="mailto:dirk@hohndel.org" target="_blank">dirk@hohndel.org</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Fri, Dec 12, 2014 at 05:03:54PM -0200, Tomaz Canabrava wrote:<br>
> > So I'm puzzled how often we are adding bookmark events and how that<br>
> > affects performance... or is this really just a first of a series and the<br>
> > others are more performance relevant?<br>
><br>
> First of a series.<br>
<br>
</span>OK<br>
<span class=""><br>
> > Dear C expert... I am curious. Why are you doing this with a struct event<br>
> > **<br>
> > instead of just a struct event *... you need the ** if you want to be able<br>
> > to modify the list in an elegant way, but all you do here is walk the<br>
> > list, so this could drop one level of indirection... or am I missing<br>
> > something?<br>
><br>
> Copied from the code just above that one.<br>
<br>
</span>But that code modified the list, right?<br>
<span class=""><br>
> > And why add the curly braces, anyway?<br>
><br>
> I was debugging and forgot to remove them.<br>
<br>
</span>Tsktsktsk. So you added something because you forgot to remove it and<br>
introduced whitespace damage that way...<br>
<span class=""><br>
> > This could do something really bad if for some reason<br>
> > internalEvent->time.seconds is negative, right? then entry[0].sec (which<br>
> > should always be 0) could already be bigger and we access element entry[-1]<br>
><br>
> Is that possible?<br>
<br>
</span>Off the top of my head I'd say "unlikely", but I worry about dives<br>
imported from other sources or something else that could break this<br>
assumption. So I'd rather have you test for i > 0 before you access<br>
entry[i - 1]<br>
<span class=""><br>
> > So this is pretty invasive and the patch has a few issues.<br>
> ><br>
> > Why should this go in before 4.3? Is there a specific bug this addresses?<br>
> ><br>
><br>
> I was trying to find out why when moving a node for a few seconds while<br>
> adding a new dive used 100% of the CPU in release mode. turned out it was<br>
> because of too many replots that used the *very* expensive calls to<br>
> PainterPath.<br>
> while I was trying to fix that I got carried away.<br>
<br>
</span>I hear that happens. So does this mean "I'll rework this for until after<br>
4.3" or "I'll rework this to address the issues mentioned and then submit<br>
the series that fixes the bigger problem later today"?<br></blockquote><div><br></div><div>After 4.3 I'll touch this again, you can forget this one.<br>:)<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> I think I'll just<br>
> generate the pixmap of the Glyphs and reuse them in the future for the<br>
> PainterPath issue.<br>
<br>
</span>Or that and we forget the patch? That seems wrong because the idea in the<br>
patch seems correct.<br>
<span class="HOEnZb"><font color="#888888"><br>
/D<br>
</font></span></blockquote></div></div></div>