Re: Undo done d'oh? metricsview.c
Dave Crossland <[email protected]> Sat, 21 Mar 2015 00:59:19 -0400
| Newsgroups | gmane.comp.fonts.fontforge.devel |
|---|---|
| Message-ID | <CAEozd0yEdBTfhEkcCJqN7Dm1f2P3Ma76GBO7HxtJg7m=e-zpCg@mail.gmail.com> |
Hi I think Ben Martin (@monkeyiq) worked on the undo system, as part of collab stuff On 20 March 2015 at 22:34, Thomas Shinnick <[email protected]> wrote: > I'm investigating undo/redo in general and wanted to look at FF's > implementation, and almost immediately started wondering if I was reading > correctly. > > Would Adrien/Frank (most recent workers there) please check if I'm crazy? > I haven't tested (would be hard to do?) but the source just looks wrong. > > FF has two different routines in fontforge/cvundoes.c that capture *character > widths* for undo/redo: SCPreserveWidth() for horizontal character width > and SCPreserve*V*Width() for vertical width. > > fontforgeexe/metricsview.c calls both routines at times, but it seems that > sometimes it is calling SCPreserveWidth() (presumably the older routine?) > when it should be calling SCPreserve*V*Width(). > > Specifically, at line 1305 > <https://github.com/fontforge/fontforge/blob/master/fontforgeexe/metricsview.c#L1305> > in routine MV_RBearingChanged() there is a section for when mv->vertical, > and similarly to the horizontal clause above, capturing undo information is > done before the change is done. But exactly the same as seen in the > horizontal clause above, SCPreserveWidth() is called. It sure seems like > it should have been SCPreserve*V*Width(). > > Likewise at line 4488 > <https://github.com/fontforge/fontforge/blob/master/fontforgeexe/metricsview.c#L4488> > we're in a section dealing with vertical values, _MVSubVMouse(), but > SCPreserve*V*Width()is not used. > > Hmm, just grepping metricsview.c for the two routine names finds 5 calls > to SCPreserveWidth() but only one call to SCPreserve*V*Width(). That > alone seems suspicious. > > > > ------------------------------------------------------------------------------ > 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 > > -- Cheers Dave ------------------------------------------------------------------------------ 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