[vtkusers] vtkChartXY

[Hagen Mölle]

Hi Marcus,

I added a new topic branch 'chartlogscale' to gerrit. I already 
corrected some remarks you did.

Hagen
> Hi Hagen,
>
> I was away at a meeting, and am getting caught up now...
>
> 2010/11/8 Hagen Mölle<h.moelle at googlemail.com>:
>> Hi Marcus,
>>
>> did you have time to look at my code. It would be nice to get some feedback
>> (maybe I should recode some part or there is something I forgot to
>> implement).
> I would love it if you uploaded the changes to Gerrit, so that I could
> give you some line-by-line comments where appropriate. There are
> trailing whitespaces that were introduced, these are easy to clean up.
> Your code often exceeds 80 characters, we ask that you split lines
> like that. Lines like,
>
> else if ((((this->LogScale&&
> !this->LogScaleReasonable)?1.0:this->TickInterval) == 0.0 ||
> this->TickInterval == -1.0))
>
> Should be split, and have space around the ? and : in the ternary.
> That is quite a nested else if, and I would prefer we revise it to
> make it clearer what it is intended to do. There are also lines such
> as,
>
> // If the values min and max are not within limits we set defaults
> +  if (min<  1.0) min = 1.0;
> +  if (min>  9.0) min = 1.0;
> +  if (max<  1.0) max = 9.0;
> +  if (min>  9.0) max = 9.0;
>
> They violate the VTK code style guidelines, it must be,
>
> if (min<  1.0)
>    {
>    min = 1.0;
>    }
>
> I assume the final line should have been max rather than min in that
> block? It seems like they are mutually exclusive and an else if would
> be more appropriate for the two pairs (if min is less then 1 it cannot
> also be greater than 9. There are C style casts, where we require C++
> casts (static_cast, etc).
>
> I see you are taking bool and int values by reference. To add this we
> would also need to add a test, which would be great for me to see the
> new code in action as well as ensure it continues to work. I would
> like to continue working on this, and get it into the charts - it has
> been on my todo list for quite some time and it looks like you have
> made a great start.
>
> http://www.vtk.org/Wiki/images/8/81/GitVTKCheatSheet.pdf
>
> The above shows you some of the steps to get this into a topic branch,
> and uploaded to Gerrit. I could provide more specific feedback there,
> and even make additional commits to fix up some of these issues. It
> will also ensure it stays in my list of things to do. Thanks for
> working on this, I look forward to adding this to the charts - I think
> it will be a well received feature addition.
>
> Thanks,
>
> Marcus
>
>>> 2010/10/26 Hagen Mölle<h.moelle at googlemail.com>:
>>>> Hi Marcus and Eric,
>>>>
>>>> I am finished with writing a new logarithmic scale for logarithmic axis.
>>>> I
>>>> produced a patch too (git diff>    VTK.patch). Is there a specific
>>>> procedure
>>>> to merge the code into current master of VTK. Before doing this you
>>>> hopefully want to look over the code and test it.
>>>>
>>>> Some remarks to my changes:
>>>>
>>>> 1. If axis labels are large then they will overlap.
>>>> 2. If order of magnitude of the axis is lower than/equal 5 then labels
>>>> will
>>>> be ... 1 2 5 10 20 50 100 ... Other wise labels will be ... 1 10 100 1000
>>>> ...
>>> I don't see any attachment. I am in the process of writing something
>>> up on using it, but Gerrit will be a preferred method of contributing
>>> changes to VTK. You can push a topic branch, I can easily fetch it
>>> into my local tree, test it and comment on it. For now, I would
>>> suggest being very brave and testing it out, using 'git format-patch'
>>> to generate a patch file, or pushing a branch on one of the services
>>> such as Gitorious.
>>>
>>> Marcus
>>> --
>>> Marcus D. Hanwell, Ph.D.
>>> R&D Engineer, Kitware Inc.
>>> (518) 881-4937
>>