[vtkusers] vtkChartXY

[Marcus D. Hanwell]

Thanks Hagen, I will take a look later. There are some other axis bugs
I need to look into.

2010/11/16 Hagen Mölle <h.moelle at googlemail.com>:
> 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
>>>
>
>