Hi Thomas,

On September 26, 2010 11:58:11 am T. Modes wrote:
> Yes, I'm sure, your changeset is the culprit.
> Changeset 22828bb9df09 works ok, but changeset d14b0775fbfa crashes.

You may be sure but I am not so sure - I had this kind of symptoms before 
applying this changeset.  They appeared a few days ago.


> > > 1.) First you did not consider the points in the other thread which
> > > were mentioned for bigger projects.
> > 
> > Yes I did.  This is why the user can enable/disable the behavior via
> > preferences.  Other kind of considerations would have taken more time
> > than I had to spare.
> 
> You did not consider the critics. The changeset is the same as your
> patch from last year. You added only a preference, but did not
> consider the critics points.
> Your code can go into an endless loop (as James already mentioned
> http://groups.google.com/group/hugin-ptx/msg/6752a969d852a97a ).
> Only when somebody can deactivate it by select an option, which would
> otherwise make it unusable, this a would *not* consider mature. Such
> "hidden" option gives hugin the reputation of an expert only program.

How do you know better than me what I have considered?  Please respect that 
there can be (and often are) different ways of considering the same critics 
and that the result of me considering the feedback can be (and in this case 
is) different than yours.

IMO it is enough consideration to activate the feature only on deliberate user 
request / preference.

I consider mature something that works as intended.  IMHO it does not have to 
protect the user from tripping over once he knowingly activated it.  

For example, if a user does not know how to drive a car, he should take 
driving lessons, not ask for cars to be dumbed down.


> If you can't fix it, than leave it in the current state

done. changeset reverted.

hg backout --merge 4416

> > > 3.) You are using ImageOptions. This structure is deprecated since
> > > merging of the layout mode and should not used any more. Exisiting
> > > parts, which still use it, needs to be change.
> > 
> > Is this documented somewhere?  If you want to deprecate structures, and
> > motivate contributors to use some structures and not others, we need an
> > easy to find documentation of this.
> 
> It is in the documentation: look at SrcPanoImage::getOptions(), the
> function you are using:
> http://hugin.sourceforge.net/docs/html/classHuginBase_1_1SrcPanoImage.html#
> a19 There stands: "Do not use: "


line 291 of a 632 lines (that's roughly 20 pages) dry and boring html page 
that is itself hidden as a link in a long list of cryptic names on the third 
or fourth hierarchy level of a documentation that states on its own homepage 
[0] that it is an incomplete work in progress and that probably encompasses 
hundreds of pages?

we need to do better than this if we want occasional contributors to actually 
adopt the conventions.

I never read that documentation.  I barely recalled it existed and probably if 
I would start by reading it my enthusiasm for hacking on the code would die 
instantly.

The way I operate is that once I have an itch I want to scratch I navigate the 
code with

find . -exec grep -l "whateverIlookfor" {} \;

Searching for a unique enough GUI term leads me to the XRC file.  Searching 
for the related term in the XRC file leads me to the parts in the code where 
it is used and where it is implemented.  I read and try to understand the few 
related functions.  I do m changes by example, looking at similar things in 
neighboring code.  I just hack.  My changes are small hacks.  I am not a 
coder.  I don't have the necessary skill and time.  I learn as I go and become 
comfortable at navigating the source code, but I also tend to forget when I am 
not in the code regulary, and unfortunately this happens often.

I need the executive summary of what can / can't should / should not be done 
with the code - something that does not take more than 15 minutes to read and 
understand.

 
> There are more points, which you don't consider, otherwise we would
> not have this discussion.

Please enlighten me.  This is why I usually seek feedback for my contributed 
patches, and I am sorry I did not do it this time.


> The list, which images are cached, should stored inside the image
> cache and not in the panorama.

where do I find the image cache?

 
> > > I think, this problem should be solved in a more general approach, as
> > > already discussed in the other thread. Otherwise it creates more
> > > problems/confusion.
> > 
> > If somebody has the time and skill to solve it in a more general
> > approach, go for it.  In the meantime, like one year ago, this patch
> > makes *my* life easier and I wanted to share it with others.
> 
> Nice, that is makes *your* life easier, but it produces more trouble
> for me.

can you please make me aware of the trouble it produces for you?

undo/redo as it was until a few days ago produced trouble for me and I solved 
that trouble.  

Interestingly the solution was a very easy one (even for me), although in 
another context you stated that "this does not work" and that "my wish would 
require a complete rewrite of the CommandHistory and all
commands." which it did not?

Yuv


[0] http://hugin.sourceforge.net/docs/html/index.html

Attachment: signature.asc
Description: This is a digitally signed message part.

Reply via email to