[vtk-developers] Bug introduced in OpenGLContextDevice2D::DrawPoly

[Eric E. Monson]

On Dec 31, 2010, at 4:30 PM, Marcus D. Hanwell wrote:

> Hi Eric,
> 
> I will have to take a better look at the code, but it looks like,
> 
> painter->DrawPoly(line[0].GetData(), static_cast<int>(cols),
>                           this->Colors->GetPointer(nc), nc_comps);
> 
> Was passing a valid color pointer, at the right point in the array,
> and the new code is simply copying that color data into a temporary
> array, passing it and then you forgot to delete the array you created.
> I would like to avoid the temporary if that is possible - and use the
> color pointer you originally used.

Sorry I forgot to delete the array. As you state below, if I pass the single color pointer the OpenGL drawing routine will just walk into improper colors since I only have one color per line that I've mapped from the table column to color by through the LUT. I don't see any other way to do it than to create temporary copies unless the DrawPoly routine is changed to accept a single color value.

> Reading the color mapping code for parallel coordinates more
> carefully, it seems that you have an array that you map to colors, and
> this nets us a single color per total line. The rendering code is
> applying a color per vertex in the line, and so is walking off the end
> of the color array. Shouldn't this code just apply a color per line
> drawn?

The old implementation of vtkOpenGLContextDevice2D::DrawPoly, after I added the color-by-array stuff, didn't really use GL_COLOR_ARRAY at all, so it was fine to just pass it a single color that would be applied to the whole line. Now, since you changed the DrawPoly routine to use a real color array I think I need to pass it an array the same length as the vertex array with multiple copies of the correct color. As you say, before it was getting passed a pointer to a single color, and then it was just walking off the end of the array during the drawing call. This didn't happen before because the DrawPoly routine just expected and used a single color per line. It's nice that it now has the flexibility to handle multiple colors per line efficiently, but some change is necessary to vtkPlotParallelCoordinates.

> I am glad you have found this work so useful, it has been good having
> such an active user and contributor to work with. We should get this
> straightened out, but it may have to wait until next year.

That's fine if it takes a bit to resolve it correctly, but there is no doubt that something needs to change about the way the PC plot is passing colors with the new way you've implemented DrawPoly.

Thanks a lot,
-Eric

> 
> Happy New Year!
> 
> Marcus
> 
> On Fri, Dec 31, 2010 at 4:13 PM, Eric E. Monson <emonson at cs.duke.edu> wrote:
>> Okay, I think I tracked it down. vtkPlotParallelCoordinates::Paint was not passing a real color array to the DrawPoly routine, only a single color value, so when you changed vtkOpenGLContextDevice2D::DrawPoly to use GL_COLOR_ARRAY the "color array" wasn't what it should have been.
>> 
>> I just pushed a change to gerrit which may fix the issue. Since it's just a little patch, feel free to re-implement it any way you want if the details of the way I've coded it aren't to your liking.
>> 
>> http://review.source.kitware.com/655
>> 
>> Well, I almost got out of 2010 with no local mods! :)  Really, the only reason things have moved so far this year for me is because of your work with the Charts, in setting up the gerrit review process and your willingness to spend time helping me and incorporating changes that I need. I really appreciate all of your efforts.
>> 
>> Happy New Year!
>> -Eric
>> 
>> On Dec 31, 2010, at 3:16 PM, Eric E. Monson wrote:
>> 
>>> As I think about it I wonder if the change you made to use GL_COLOR_ARRAY enabled instead of my implementation which used glColor4ubv with the vertex array to draw line strips in DrawPoly is just revealing a problem somewhere else (e.g. vtkPlotParallelCoordinates) rather than creating it...
>>> 
>>> If one is drawing GL_LINE_STRIPs with GL_COLOR_ARRAY enabled, does the color array need to have one less entry than the vertex array (since there is one less line segment than there are vertices)?
>>> 
>>> -Eric
>>> 
>>> On Dec 31, 2010, at 1:08 PM, Eric E. Monson wrote:
>>> 
>>>> Hey Marcus,
>>>> 
>>>> I'm getting closer to tracking down the problem I've been having with my parallel coordinates plots drawing some of the lines wrong when color-by-array is on: I think it's something that was introduced with commit 59f3d82d9183d7b3c5f014dfc7a3159562e0e494 when you "Split out the OpenGL 2 code from legacy OpenGL" and reworked some of the routines.
>>>> 
>>>> Attached are a Python test and two images, before and after that commit (16061efb087a07b22235d5cd1417b0cfe871218c used for "before"). It's like the color mapping has been shifted by a couple of indexes so that some parts of line segments are being drawn with the color that should be for later segments. (I know that's a bad description, but look at the images and see if you can see what I mean...)
>>>> 
>>>> I'll start taking a look at the code pretty soon, but I wanted to point it out in case an idea pops into your head right away.
>>>> 
>>>> Thanks,
>>>> -Eric
>>>> 
>>>> <test_pcoords_cats.py>
>>>> <Before.png>
>>>> <After.png>_______________________________________________
>>>> Powered by www.kitware.com
>>>> 
>>>> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>>>> 
>>>> Follow this link to subscribe/unsubscribe:
>>>> http://www.vtk.org/mailman/listinfo/vtk-developers
>>>> 
>>> 
>>> _______________________________________________
>>> Powered by www.kitware.com
>>> 
>>> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>>> 
>>> Follow this link to subscribe/unsubscribe:
>>> http://www.vtk.org/mailman/listinfo/vtk-developers
>> 
>>