Line attributes in lego1 & lego2

Zesp · May 31, 2013, 10:27pm

[quote]This is the best we can do for now without deep changes I reckon.[/quote]Unless a DrawFillAreaWithBorders() method exist or be implemented.
Unfortunately this does not seem to be the case

// TVirtualX // Semi-Abstract base class defining a generic interface to the // underlying, low level, graphics system (X11, Win32, MacOS).
Cheers,
Z

Zesp · June 2, 2013, 5:49pm

Hi again,

In order to be complete attached’s an extension of the last patch that also:

handles line attributes for surfaces (all modes)
canvas_stack0_plotlegosurf_systcar.pdf (134 KB)
takes into account all coordinate systems (car, pol, cyl, sph, (and psr))
canvas_stack0_plotlegosurf_systall.pdf (524 KB)even for stacks (lego only since stacks are not (yet?) supported for surfaces).
canvases.pdf (1.6 MB)(26 canvases in all).

The archive ztest.tgz contains the programs used to generate these outputs.
Run ztest.max (it will compile and execute stack0.cpp & stack1.cpp).

The diff files attached (THistPainter.cxx.diff & TPainter3dAlgorithms.cxx.diff) were generated against trunk rev 2fa236059378392c87f53e4821adb7593ad3955e.
So TPainter3dAlgorithms.cxx.diff cancels and replaces TPainter3dAlgorithms.cxx.3.diff above.

Enjoy!
Z
TPainter3dAlgorithms.cxx.diff (17.1 KB)
THistPainter.cxx.diff (1.61 KB)
ztest.tgz (3.6 KB)

couet · June 10, 2013, 6:19am

Hello,

I was away last week. Give me time to recover my emails and I will look at this.

Olivier

Zesp · June 17, 2013, 9:40am

A bit of time this week?
Cheers,
Z

couet · June 17, 2013, 12:54pm

Yep, but this THistPainter patch is not appropriate now. I get:

imaccouet:roottrunk couet$ patch  -p1 < /Users/couet/Downloads/THistPainter.cxx.diff
patching file hist/histpainter/src/THistPainter.cxx
Hunk #1 succeeded at 6604 (offset 10 lines).
Hunk #2 succeeded at 7768 (offset 15 lines).
Hunk #3 succeeded at 7845 (offset 15 lines).
Hunk #4 succeeded at 7929 (offset 15 lines).

Can you provide a diff against the latest version ?
Thanks

Zesp · June 17, 2013, 1:51pm

Here we go.
Could you please run ztest.max (see ztest.tgz above) and make sure you get the same plots as above?
Cheers,
Z
THistPainter.cxx.5.diff (1.61 KB)

couet · June 17, 2013, 1:56pm

I will

couet · June 17, 2013, 2:25pm

Ok it seems fine. Sorry to ask you but I lost a bit the tracks of your mods now. Can you provide me a small
piece of text for the release notes explaining what this patches are supposed to do ?

also, In you code I see:

   for (i=0;i<fNStack;i++) { fColorMain[i] = 1; fColorDark[i] = 1; fEdgeColor[i] = kMagenta; fEdgeStyle[i] = 10; fEdgeWidth[i] = 10; }  /// default foolish values for errors to be spotted easily

That’s sounds a bit like a dirty trick, specially with a just comment … I do not like this too much …

Zesp · June 17, 2013, 2:43pm

[quote]Can you provide me a small piece of text for the release notes explaining what these patches are supposed to do ?[/quote]This patch allows to take into account line attributes (color, style, width) when drawing a TH2 in lego or surf mode whatever the coordinate system used (car, pol, cyl, sph, (and psr)). It also handles THStack (lego only).

[quote]for (i=0;i<fNStack;i++) { fColorMain[i] = 1; fColorDark[i] = 1; fEdgeColor[i] = kMagenta; fEdgeStyle[i] = 10; fEdgeWidth[i] = 10; } /// default foolish values for errors to be spotted easilyThat sounds a bit like a dirty trick, specially with a just comment … I do not like this too much …[/quote]Not at all. These were just the default values I used to make sure eveything was going fine i.e. no apparition of huge (width=10) “dotted” (style=10) magenta (color=kMagenta) lines on screen. You can set everything to 1 and remove the comment safely.

Cheers,
Z

couet · June 17, 2013, 2:49pm

You can set everything to 1 and remove the comment safely.

I will go that way. That kind of comment becomes very quickly very obscure with time
and does not help developers coming after.

couet · June 17, 2013, 3:50pm

Patch applied. Let me know if ok.

couet · June 18, 2013, 2:27pm

Your last mods generate a big bug in surface plotting.
See the result of the SURF3 option in the attached plot.
Can you fix that please ?
sg35.pdf (152 KB)

couet · June 18, 2013, 3:44pm

the way to reproduce it is:

TH2D * histo = new TH2D ("histo","histo",20,-5.,5.,20,-5.,5.) ;
histo->FillRandom("gaus",1000) ;
histo->SetStats(kFALSE) ;
histo->SetFillColor(kRed) ;
histo->SetLineColor(kBlack) ;
histo->Draw("surf3");

couet · June 18, 2013, 4:11pm

Ok, now fixed.

Zesp · June 18, 2013, 8:56pm

Actually I could not reproduce your problem: the 2D col plane always lies above the 3D plot in my case (your example and all the plots provided above).
But if you managed to fix it in your case, everything’s fine.
Note that the change made (I never touched this part!) should be corrected though. A semicolon rather than a comma (between the assignments) would be better I reckon (see lines 114 & 198).
Cheers,
Z

couet · June 19, 2013, 7:33am

You are right. Now changed.

Zesp · June 26, 2013, 9:06pm

Hi Olivier,

I’m a bit disappointed these changes
(and others [url]TH1::Integral(“width”) in TPaveStats (statbox) & [url]No shadow in lego1
did not get implemented in the last ROOT patch release (v5-34-09)
(I’m very happy they got commited to the trunk though).
Should I provide the corresponding patches against the head of the “v5-34-00-patches” branch
to see them included in the next patch release (v5-34-10)?

Cheers,
Z

couet · June 28, 2013, 7:52am

Yes

Zesp · July 1, 2013, 9:55pm

Here we go (diff against heads/v5-34-00-patches@v5-34-09-10-g2ed83fa)
It holds for the 3 posts abovementioned (attline for lego & surf, lego4 and TH1::Integral(“width”) in TPaveStats) since all bring in THistPainter.cxx.
Checks passed using the different test macros provided.
Cheers,
Z
patch6.tgz (8.3 KB)

Zesp · July 2, 2013, 7:04am

Please note patch6.tgz is a terrible name since it is not meant for the master branch.
It should be read patch-v5-34-00-patches.tgz.
The names inside the archive are properly named.
Cheers,
Z