Allow line color different from axes and grids in 3D plots

[mars0i]

For me it would be useful to have an optional ~color argument for mesh and surf, like the optional argument for the 2D plotting functions, so that axes and grids can be a different color from plot lines. Maybe this makes more sense if new features are implemented for issue #57, as illustrated in this example: foo.pdf. I can submit a PR if this seems OK.

[mars0i]

I've added a ~color parameter to mesh in my fork here. I'm planning to do the same thing for surf.

There was an unexpected problem, though. The axes and tick marks had the color specified by set_foreground_color, but the grid lines that run behind the plot had the same color as the plot, i.e. the color specified by the ~color argument to mesh.

A solution is to use plcol0 index 2 instead of 1, which is what's used throughout owl_plot.ml. This is what I've done in my fork: https://github.com/mars0i/owl/blob/master/lib/owl_plot.ml#L257
This seems weird to have to do this. Everywhere else in owl_plot.ml, the code just uses index 1, and saves and restores the rgb values to col0 index 1 as needed. However, using index 1 in _prepare_page results in the grid lines having the color of the plot, while using index 2 in _prepare_page causes the grid lines to have the same color as the axes and tick marks.

The axes, ticks, and grid lines are drawn by plbox3. However, plbox3 draws the axes and tick marks immediately (i.e. during the execution of _prepare_page), but doesn't draw the grid lines until the time that the plot is drawn. The documentation for plbox3 says "These lines are not drawn until after plot3d or plmesh are called because of the need for hidden line removal. "

plbox3 apparently remembers the col0 index that was in scope when plbox3 was called, and gets the RGB value from that col0 index at the time of plotting to choose the color for the grid lines. So the col0 index at the time of the plbox3 call has to be different from the one used for the plotting color.

[ryanrhymes]

This is very useful so please do submit your PR, really appreciate it.

Thanks for digging into PLPLOT doc, have you tested whether "changing into plcol0 2" affectc other plot functions or does this change their original behaviours?

[mars0i]

It seems like it should not change the behavior of the other functions, but I know that I need to test it. I was planning to try creating PDFs from all of the examples in the tutorial, with and without the plcol0 2 modification to _prepare_page. My bet is that the resulting PDF files will be binary identical. (I don't think the Cairo driver puts timestamps or other inessential fields in the PDF.) Would that be enough of a test, do you think?

[ryanrhymes]

It seems like it should not change the behavior of the other functions, but I know that I need to test it. I was planning to try creating PDFs from all of the examples in the tutorial, with and without the plcol0 2 modification to _prepare_page. My bet is that the resulting PDF files will be binary identical. Would that be enough of a test, do you think?

Sure, that is definitely enough imo. Many thanks Marshall 👍

Also fyi, the wiki should be publicly editable now after changing the github repo setting.

[mars0i]

Colors set with plcol0 seem to be ignored by plsurf3d, so I removed the color parameter I tried adding to surf. Maybe there's a way to control the color in the way that other plots are controlled, but I think plsurf3d may be different. My theory about this is that single-color specifications don't make sense because the surface plots are designed to be colored by magnitude or using the light source (which can be changed using function pllightsource).

[mars0i]

f it's OK to submit a PR with both the color changes and the addition of the optional plplot option argument for mesh and surf, my fork has all of these changes in the same branch. If you'd rather have two separate PRs for the color argument changes and the plplot option argument changes, it wouldn't be difficult to reimplement the color changes on a fresh clone of your main branch. (I still have to run the Tutorial 4 tests first.)

[ryanrhymes]

I definitely would love to incorporate this feature. I am currently refactoring the code in Plotplot to adapt to the new API design I mentioned before. It will take a couple of days time.

So how about letting me finish that first, then you can adapt your code to the new API? It may also solve the plclol0 problem at the same time.

[mars0i]

Oh--great. Sure, I'll wait.

[ryanrhymes]

@mars0i The API of Plot module has been redesigned. Every high-level function now has an optional spec parameter to pass in various specification to control the plotting behaviours. This not only significantly simplifies the API but also gives us the greatest flexibility to add more control parameters (without changing function signature) in future to enhance the functions.

If you pass in some parameter that a function does not understand, it will be ignored. I have updated the tutorial accordingly and I hope you it will help you in understanding the new design. Let me know if you notice any mistakes. https://github.com/ryanrhymes/owl/wiki/Tutorial:-How-to-Plot-in-Owl%3F

The new design may require you to change your current code but should not be significant. My apologies for the inconvenience.

[mars0i]

Thanks Liang. Just starting to look at it. Seems like a very nice design. I don't mind having to change my code, though; I appreciate the value of rationalizing design after an early period of code growth.

And it's partly my fault. :-)

[mars0i]

I added this feature to the new, refactored version. It wasn't hard to modify my project code for the refactored Owl, and then I decided to add in the plot draw vs. axes/grid lines color difference in one of my branches.

It's in this file. The changes are in _prepare_page and mesh: mars0i@d5bb58e?diff=split

I didn't do anything similar in surf because plcol seems to have no effect on the PLPlot function that surf uses.

The change doesn't affect any of the plot examples that were in the tutorial up until a day or two ago. (Nice additions btw.)

If a PR would be helpful, I'm happy to submit one, and you can make any changes you want of course. If you have a different idea for how to do this but want to just copy or rewrite a few of my lines by hand without a PR, that's fine, too, of course. You may want to do something completely different, though.

(BTW I do have an example that would be nice for the tutorial, but the data is generated by a bunch of my own project code, and I think there's too much code in that that will be confusing. I need to come up with a simple way to make some fake data that will illustrate the same idea. That will probably have to wait for a couple of weeks.)

[ryanrhymes]

Thanks, this doesn't require significant changes, so I just reused some of the code you provided. It is already in the master branch.

[mars0i]

Great--glad it was helpful. Thanks.