2012/2/19 Krzysztof Kosiński <tweenk.pl@...400...>:

2012/2/19 Johan Engelen <jbc.engelen@...2592...>:

...

Hi all,   We have two helper directories: src/helper and src/util. I think we should merge the two. Which name do we want to keep?

Note we have for example src/helper/units.h and src/util/units.h, argh! If anyone is looking for a GSoC project: a unification of the units system would be nice. If I remember correctly, we have 4 (!) separate pieces of unit calculation code...

The vast majority of stuff in these directories is actually unnecessary.

src/helper:

action.h: can be remplaced with Gtk::Action if the icon cache system is slightly modified, so that the icon cache matches the directory structure of icon themes. The only reason this exists is because icons pulled from icons.svg need special handling. However, thanks to Jon's work on caching the icons as PNG, this is no longer necessary. geom.h: should be integrated into 2Geom. geom-curves.h: ditto. geom-nodetype.h: ditto. gnome-utils.h: replace with g_uri_list_extract_uris() and g_filename_from_uri(). pixbuf-ops.h: Rename and move to display/ png-write.h: Hack, should be implemented as a call to an output extension. recthull.h: Replace with Geom::OptRect which has equivalent functionality. sp-marshal.h: I recall that this can be replaced with a generic marshaller, which requires libffi. Not sure though. stlport.h: rename but leave in place. stock-items.h: replace with icon theme as mentioned for action.h.

Just a heads up, but stock-items is nothing to do with icons. its to do with having markers/patterns etc get picked up from the share folder and user folders.

unit-menu.h: move to ui/widget (after gtkmmification). units.h: merge into util/units.h unit-tracker.h: no idea what this does. window.h: replace with Inkscape::window_new as mentioned in comment.

src/util:

accumulators.h: Accumulators for sigc++ signals. I think this is unused at the moment. compose.hpp: This function is available in recent versions of glibmm as Glib::ustring::compose(). copy.h: huh? This should not be necessary. In case it is, replace with boost::remove_const<T>::type ege-appear-time-tracker.h: some arcane stuff that should be documented or removed. ege-tags.h: this is supposed to implement the CREATE specification for sharing resources between FOSS art programs, but at the moment it doesn't do anything meaningful. Remove. enums.h: no idea. expression-evaluator.h: code copied from GIMP that is responsible for evaluating math expressions in spinboxes. It uses GPLv3 whereas we are officially GPLv2, though almost all files are "released under GPL" without specifying a version. This makes our current codebase theoretically illegal so we should do something to fix this. filter-list.h, find-if-before.h, find-last-if.h: There should be an algorithm in STL or Boost that does the same. fixed-point.h: assuming it is actually used, it should be rewritten to take advantage of Boost operator helpers and moved to 2Geom. format.h: This is a sprintf equivalent for ptr_shared<char>. Depends on what we do with ptr_shared. forward-pointer-iterator.h: Rewrite using Boost for conciseness but leave in place. function.h: Replace with Boost. glib-list-iterators.h: must stay for now. list.h, list-container.h: replace with STL lists with GC allocator. longest-common-suffix.h: Probably needs to stay, since I don't know of anything similar in Boost. mathfns.h: should be moved to 2Geom. reference.h: replace with boost::add_reference<T>::type reverse-list.h: Same as list.h share.h: I don't know. I guess this could be replaced with GQuarks, but the crucial difference is that quarks persist until the end of program, while this is supposed to be GCed. tuple.h: replace with Boost. ucompose.hpp: see compose.hpp. units.h: needs to stay unordered-containers.h: needs to stay

So the following files are needed: helper/stlport.h helper/unit-tracker.h util/enums.h util/expression-evaluator.h util/forward-pointer-iterator.h util/glib-list-iterators.h util/format.h util/share.h util/units.h util/unordered-containers.h

I think it should therefore be easier to remove the 'helper' directory.

Reards, Krzysztof

---

Virtualization & Cloud Management Using Capacity Planning Cloud computing makes use of virtualization - but cloud computing also focuses on allowing computing to be delivered as a service. http://www.accelacomm.com/jaw/sfnl/114/51521223/ _______________________________________________ Inkscape-devel mailing list Inkscape-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/inkscape-devel

On Feb 19, 2012, at 9:23 AM, Krzysztof Kosiński wrote:

action.h: can be remplaced with Gtk::Action if the icon cache system is slightly modified, so that the icon cache matches the directory structure of icon themes. The only reason this exists is because icons pulled from icons.svg need special handling. However, thanks to Jon's work on caching the icons as PNG, this is no longer necessary.

Minor point.

The reason also comes from legacy where Inkscape added the 'action' concept before GTK+ itself added GtkAction.

Gives you a clue as to how much some of this stuff needs a cleanup pass, huh?

:-)

On Feb 19, 2012, at 9:23 AM, Krzysztof Kosiński wrote:

unit-tracker.h: no idea what this does.

The documentation is a bit sparse, but it does have

/* * Inkscape::UnitTracker - Simple mediator to synchronize changes to a set * of possible units *

One summary of the mediator pattern is at http://www.dofactory.com/Patterns/PatternMediator.aspx

Then there's http://en.wikipedia.org/wiki/Mediator_pattern

In this context, it assists maintaining encapsulation and modularity through improving loose coupling.

On 19-02-12 18:23, Krzysztof Kosiński wrote:

2012/2/19 Johan Engelen <jbc.engelen@...2592...>:

...

fixed-point.h: assuming it is actually used, it should be rewritten to take advantage of Boost operator helpers and moved to 2Geom.

I can comment on that one, as I wrote it :) It's used in gaussian blur (or at least, it used to be), although it's used less than I initially planned for (the IIR code really seems to need the added precision of doubles, one reason I'm still looking for a replacement). It doesn't make sense to me to move it to 2geom, as it has nothing to do with geometry, but using Boost to simplify/clean up the code sounds like an excellent plan.

Also, it might be worth seeing whether it actually is still worth it in terms of speed. It should be relatively simple to make the FIR code use "float" instead of a fixed point type and see if that makes any speed difference. There is a typedef on line 66 of nr-filter-gaussian.cpp, if someone (preferably multiple someones) with a reasonable modern machine could check whether changing that typedef to set FIRValue to "float" makes a (huge) speed difference, that would be great. If not, the time might have come to simply remove this optimization.

When testing the speed of the FIR code, it helps to set "use_IIR_?" on lines 601/602 to false, then you know for sure that you're using the FIR code, and not the IIR code.

... mathfns.h: should be moved to 2Geom.

The triangle area makes sense to move to 2geom, but I'm not so sure about the other ones, they look like they might be related to grids or something. (At least I see no reason to put them in 2geom.)

Great effort btw!