Re: Undo done d'oh? metricsview.c

Amazing job as always Thomas :)
On 21 Mar 2015 20:25, "Thomas Shinnick" <[email protected]> wrote:

> No, he's not to blame a bit, nor anyone present. This really looks like
> confusion from 2004 timeframe (or is that the date when imported into
> SF/GH? Ah, code is wrong 2003-06-11 in originals at SF)
>
> SCPreserveVWidth() saves vwidth into *u.state.width*
> <https://github.com/fontforge/fontforge/blame/master/fontforge/cvundoes.c#L871>
> setting undo type ut_vwidth.  Mmm, and so does SCPreserveWidth() except
> using type
> ut_width.  Mmm, but in splinefont.h the definition is
>     union {
>         struct {
>             int16 width, vwidth;
>                     . . . . .
>         } state;
> If you are using the '*state*' section (undo types like ut_state) then it
> should be u.state.width and u.state.*vwidth* (and these are used
> correctly this way elsewhere).
>
> But then in SCUndoAct() - the implementation of undoing - in the cases
> for ut_width and ut_vwidth
> <https://github.com/fontforge/fontforge/blame/master/fontforge/cvundoes.c#L911>
> it accesses *u.width* , not u.state.width.  And u.width looks to be the
> correct usage.
>
> So, like, er, umm, this particular undo/redo usage has been broken since
> the beginning?  And these routines are called from 15 places, so there's
> likely some breakage.  (Unlike routines CVPreserveWidth() /
> CVPreserveVWidth() *which are never called*, but which *are* coded
> correctly... >.<)
>
> So, technical debt and untested code whinging, y'know?  Suppose I ought to
> debug / prove / PR ...
>
> On Fri, Mar 20, 2015 at 11:59 PM, Dave Crossland <[email protected]> wrote:
>
>> Hi
>>
>> I think Ben Martin (@monkeyiq) worked on the undo system, as part of
>> collab stuff
>>
>
>
>
> ------------------------------------------------------------------------------
> Dive into the World of Parallel Programming The Go Parallel Website,
> sponsored
> by Intel and developed in partnership with Slashdot Media, is your hub for
> all
> things parallel software development, from weekly thought leadership blogs
> to
> news, videos, case studies, tutorials and more. Take a look and join the
> conversation now. http://goparallel.sourceforge.net/
> _______________________________________________
> fontforge-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/fontforge-devel
> http://fontforge.10959.n7.nabble.com/Developer-f3.html
>
>

------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/

_______________________________________________
fontforge-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/fontforge-devel
http://fontforge.10959.n7.nabble.com/Developer-f3.html