Skip to content

Conversation

@rlalik
Copy link
Contributor

@rlalik rlalik commented Oct 28, 2025

This Pull request:

Implements feature proposed here: #20165 (comment)

Changes:

  • When nx or ny is negative, the pads is divided according to abs(nx) or abs(ny), but the outer margins is set to 0. The inner margins are equal to xmargin and ymargin, respectively. Demo of the new feature in the bottom row of the image:
image

Created with macro:

void can_test(int nx = 3, int ny = 3, int color = 46) {
    /* can_0 to can_6 here, not relevant */
    TCanvas * can_7 = new TCanvas("can_7", "can_7", 200, 664, 200, 200);
    can_7->Divide(-nx, -ny, 0.05, 0.05, color);
    can_7->Draw();

    TCanvas * can_8 = new TCanvas("can_8", "can_8", 408, 664, 200, 200);
    can_8->Divide(-nx, ny, 0.05, 0.05, color);
    can_8->Draw();

    TCanvas * can_9 = new TCanvas("can_9", "can_9", 616, 664, 200, 200);
    can_9->Divide(nx, -ny, 0.05, 0.05, color);
    can_9->Draw();
}

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

Also related to #20138

@rlalik rlalik requested a review from couet as a code owner October 28, 2025 00:23
When nx or ny is negative, the pads is divided according to abs(nx) or abs(ny), but the outer margins is set to 0. The inner margins are equal to xmargin and ymargin, respectively.
@rlalik rlalik force-pushed the canvas_divide_margins branch from ad4dab7 to 9eb99eb Compare October 28, 2025 08:09
@rlalik
Copy link
Contributor Author

rlalik commented Oct 28, 2025

I force pushed small fix. In the previous version I was generating margins twice smaller than in the original implementation. So in my implementation whenever I use xmargin or ymargin I must use 2 * x(y)margin to achieve backward compatibility. Now is fixed. I also updated the image and test macro.

couet
couet approved these changes Oct 28, 2025
@rlalik
Copy link
Contributor Author

rlalik commented Oct 28, 2025

@couet But I started to have a second thoughts. Unfortunately, my proposed solution, if run on older ROOT version, will fall for the previous behavior:

   if (nx <= 0) nx = 1;
   if (ny <= 0) ny = 1;

and if someones does can->cd(3), the code will silently crash without explanation.

So perhaps other idea would be one of two:

  1. What is the special case for xmargin and ymargin < 0? I asked a couple of time about this but never got answer. Is it really used? It could be used (and it was my initial idea) to use negative values for zero outer margins, if this case isn't really used by anyone - and I don't know why it should be used as it produces odd case.
  2. Change signature of Divide function to:
void TPad::Divide(Int_t nx, Int_t ny, Float_t xmargin, Float_t ymargin, Int_t color, Float_t xmargin_outer = 999.0, Float_t ymargin_outer = 999.0)

or make an overload:

void TPad::Divide(Int_t nx, Int_t ny, Float_t xmargin, Float_t ymargin, Int_t color, Float_t xmargin_outer, Float_t ymargin_outer)

This is more flexible as allows to adjust the outer margin - may not be 0 but may also be smaller than inner margins. If value is 1 or more (for the case with defaults), then inner x(y)margins are used as the outer margin. This will also not work with the older ROOT but at least will give compilation/execution error which user can lookup in the documentation. But this is ABI breaking change.

@couet
Copy link
Member

couet commented Oct 28, 2025

I do not think a negative marging has some meaning. It is not allowed. In SetMargin one can see this protection:

if ([margin < 0 || margin >1) margin = 0.1;

@rlalik
Copy link
Contributor Author

rlalik commented Oct 28, 2025

I do not think a negative marging has some meaning. It is not allowed. In SetMargin one can see this protection:

if ([margin < 0 || margin >1) margin = 0.1;

But documentation says something else. Please see this comment: #20138 (comment) and see the image which is just above. The lower row of canavses has odd behavior - non-zero top-right margins. I reported that in my first issue. And in the code the top-right margin is somehow treated in a special way. So perhaps this special case should be removed? Perhaps it is some very obsolete code and not relevant today?

@couet
Copy link
Member

couet commented Oct 28, 2025

Ok I need to check....

@couet
Copy link
Member

couet commented Oct 28, 2025

The margins <0 means there is now space between the pads. May be there is somthing wrong in that case. Does your PR fixes it ? That's very old code which was not change since 21 years. May be it can be improved as you suggest.

@rlalik
Copy link
Contributor Author

rlalik commented Oct 28, 2025

I tried to not modify this behavior as it was explicitly documented, so assumed that it is important to keep it as it is. So I changed from initial idea of using negative margins to negative divisions numbers. But if you say, I can get back to initial idea of negative margins, and remove the this old obsolete code.

@couet
Copy link
Member

couet commented Oct 28, 2025

Yes negative margins means no spaces betwen the pads. This code was introduced by this PR: e2cbbfc

it says:

Extend the functionality of TPad::Divide when xmargin=0 and ymargin=0.
In this case the pad is divided with no space between the subpads.
The size of each useful frame inside each pad is identical.
eg, try the following script:

void divpad(Int_t nx=3, Int_t ny=2) {
   TCanvas *c1 = new TCanvas("c1");
   c1->Divide(nx,ny,0,0);
   Int_t number = 0;
   TH1F *h = new TH1F("h","h",100,-3.3,3.3);
   h->GetXaxis()->SetLabelFont(53);
   h->GetXaxis()->SetLabelSize(10);
   h->GetYaxis()->SetLabelFont(53);
   h->GetYaxis()->SetLabelSize(10);
   h->SetMaximum(30*nx*ny);
   h->SetFillColor(42);
   for (Int_t i=0;i<nx*ny;i++) {
      number++;
      c1->cd(number);
      h->FillRandom("gaus",1000);
      h->DrawCopy();
   }
}

When you run this example it looks fine.

@rlalik
Copy link
Contributor Author

rlalik commented Oct 28, 2025

Margins set to 0 are fine. But margins set to negative number give the odd top-right non-negative margin, and left-bottom zero margin. See the middle row of the image on top of the page.

@couet
Copy link
Member

couet commented Oct 28, 2025

In the example above (given in the original PR) if i put negative margins like:

c1->Divide(nx,ny,.1,-1);

I get exactly the same plot as the original with margins = 0.

@rlalik
Copy link
Contributor Author

rlalik commented Oct 28, 2025

I took your example and made only this change: c1->Divide(nx,ny,1,-1, 38); to add color to subpad and set negative margin:

void divpad(Int_t nx=3, Int_t ny=2) {
   TCanvas *c1 = new TCanvas("c1");
   c1->Divide(nx,ny, 1, -1, 38);
   Int_t number = 0;
   TH1F *h = new TH1F("h","h",100,-3.3,3.3);
   h->GetXaxis()->SetLabelFont(53);
   h->GetXaxis()->SetLabelSize(10);
   h->GetYaxis()->SetLabelFont(53);
   h->GetYaxis()->SetLabelSize(10);
   h->SetMaximum(30*nx*ny);
   h->SetFillColor(42);
   for (Int_t i=0;i<nx*ny;i++) {
      number++;
      c1->cd(number);
      h->FillRandom("gaus",1000);
      h->DrawCopy();
   }
}

The result is following:
c1
Can you past image generated by this exact figure on your system? You are plotting white backgrounds so you may not be able to notice the odd margins.

@ferdymercury
Copy link
Collaborator

the odd margins

I guess it leaves as much margin on the right part as the space on the left for the y axis labels? Same for top vs bottom x labels. So that the axes lines are centered.

@couet
Copy link
Member

couet commented Oct 28, 2025

This is done on purpose. This divide allows to have the exact save drawing area for all the pads. The right and top margins (green in your case) compensate the left and bottom margins in which the axis are drawn. That's what "The current pad margins are recomputed to optimize the layout" means. This avoid the effect you get with 0 margins in which the drawing area are not all the same:

void divpad(Int_t nx=3, Int_t ny=2) {
   TCanvas *c1 = new TCanvas("c1");
   c1->Divide(nx,ny,0.00001,0.00001);
   Int_t number = 0;
   TH1F *h = new TH1F("h","h",100,-3.3,3.3);
   h->GetXaxis()->SetLabelFont(53);
   h->GetXaxis()->SetLabelSize(10);
   h->GetYaxis()->SetLabelFont(53);
   h->GetYaxis()->SetLabelSize(10);
   h->SetMaximum(30*nx*ny);
   h->SetFillColor(42);
   for (Int_t i=0;i<nx*ny;i++) {
      number++;
      auto p = c1->cd(number);
      if (i!= 0 && i!=3) p->SetLeftMargin(0.0);
      p->SetRightMargin(0.0);
      if (i<3) p->SetBottomMargin(0.0);
      p->SetTopMargin(0.0);
      h->FillRandom("gaus",1000);
      h->DrawCopy();
   }
}
Screenshot 2025-10-28 at 16 24 15

This divide (with negative margins) allows to get easily common axis for all pads making sure the drawing area has the same size in all pads.

@couet couet self-requested a review October 28, 2025 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants