Baselines for ENH: Add vector legend. #169
Conversation
|
@doutriaux1 Also take a look that the changed baselines and how the vector legend looks like. Note that for linear mapping we have two vectors in the legend for other scaling options only one vector is sufficient. |
|
@danlipsa I think we shouldn't draw the legend's box. |
I used drawLinesAndMarkersLegend as a starting point, and that function draws a box - this way it is consistent. Thinking about this, I kind of like the box with an arrow inside, instead of just an arrow floating without any context.
Note that you can also have two arrows which are drawn at 1/3 and 2/3, so I think centered fits well with this. If you feel strongly we should change these we can get @aashish24 opinion as well. However, he might prefer something different than both of us and then we are stuck 😄 |
can we make it optional with default to no-box? |
I like the center since there it is noticeable. If it is left aligned, i would have missed it. My vote is for center |
|
@doutriaux1 @aashish24 I can just delete the box then. |
|
No box by default as before can be useful, in case you already have a boxfill/isofill or some other gm legend, and you need to find some space to put all this on the canvas btw, what happens if your template.legend box is higher than it is wide. Do you get a vertical arrow? Though I think a vertical arrow may look strange... |
|
@jypeter For now, I only have the horizontal layout. @doutriaux1 Do we have vertical layouts for other legends (color bar, line plots)? @doutriaux1 OK, I'll remove the box. Should I still keep it as an option, or completely remove it? |
|
yes completely remove it. |
|
@danlipsa I see that isoline does not draw a legend anymore... I'll dig the old vcs |
|
|
|
@danlipsa @aashish24 this is the same plot in the latest vcs |
|
@aashish24 it is not even close in quality 😿 |
|
@aashish24 @danlipsa latest png generate on mac/mesa with default settings |
|
@danlipsa let's not worry about the legend as long as you removed the box. I need to get this release out! |
|
@doutriaux1 Sounds good. I'll just remove the box and leave everything else as is. |
|
@doutriaux1 @aashish24 Updating baselines is such a pain with the new testing framework = while (forever)
run test script
update one baseline
Compare this with before =
We should really fix this: |
this is the comparison with vector graphics and raster graphics. A fair comparison would be if you export both as vector format 😄. The underlying technology has its up and downs. Vector looks better by default because you get infinite resolution, raster is faster and hardware accelerated. If you export both as PDF they should look the same. |
+1, I like the legend at the center since it is only one arrow but I think that should be a user option since this is really a matter of choice, nothing is right or wrong. |
@doutriaux1 I thought we may not get the legend in the release as discussed over the phone call. I guess since we got extra time now, are we planning to have it in the release? If that is the case @danlipsa you may want to add a notebook example after the branch is merged. |
|
@danlipsa there is an update keyword now in the checkImage code, so if you know what you're doing you can force update all the blines. |
|
@aashish24 this is a fair comparison, they're both pngs. |
@doutriaux1 yes they both are png's but one came from vector graphics and other from raster graphics (http://vector-conversions.com/vectorizing/raster_vs_vector.html). The output from vector graphics to png will have aliasing applied and that's why you do not see the sharp lines but rather a smooth looking plot. If you want to do the same, then enable the aliasing and it should get better. By default we turn off aliasing. |
|
@doutriaux1 @aashish24 I agree the old vcs looks better. Things we can do:
We can file an issue and fix this. |
|
+1 for what @danlipsa said. I also notes some other differences in defaults. I think it makes sense to turn off aliasing for testing but we should have anti-aliasing on as default and that will ensure that our plots by default are pretty as @doutriaux1 noted |
|
@aashish24 FYI antialiasing was return |
|
@sankhesh This is the anti-aliasing issue @doutriaux1 mentioned. |
|
@doutriaux1 I think you see differences because of isoline
|
|
I'm sorry my comment has nothing to do with vector legend, but do you have anti-aliasing or not in the 2 figures above? And is it for the whole plot or just the data part (in this case the plotted isolines and continents)? Because when I look at the text, even with the sun shining on my screen, I get the impression that is slightly different in both figures, ie the text on the 0.5 figure looks more anti-aliased than the text on the 1.0 figure Is anti-aliasing something that can be selected by the user? I totally agree that it should be 'on' by default and I understand that it's probably better to have it 'off' for testing purpose if you need to compare images. What is the anti-aliasing setting for 2.8.0? |
Yes, antialiasing is enabled for both the plots and is set to 8 MSAA samples for the whole plot.
It is 8 samples by default: https://github.com/UV-CDAT/vcs/blob/8293dd09eb798952da87cce4bce993a3ba6d8d3e/vcs/VTKPlots.py#L74 You can set it on the canvas as follows: import vcs
x = vcs.init()
x.setantialiasing(8) # 8 MSAA samples
# x.setantialiasing(0) # disable antialiasing |
@jypeter I agree with you. On the text I do see some difference in anti-aliasing for text which is sometwhat bit surprising. We will have a look at it. @sankhesh can you run your test again and post your images again? Make sure that the plots of same size and so is the PNG. Can you post it sometime soon? Also perhaps we should create an issue in VCS and follow up conversation there. @doutriaux1 is it possible that you captured the last image from testing which sets the anti-aliasing to 0? |
|
@aashish24 @doutriaux1 @jypeter @danlipsa Please continue the discussion at CDAT/vcs#190 |
|
@aashish24 nope did it from the command line and checked that antialiasing was 8 |
No description provided.