root-project / root

The official repository for ROOT: analyzing, storing and visualizing big data, scientifically
https://root.cern
Other
2.72k stars 1.29k forks source link

TCanvas size is inconsistent in batch vs non-batch mode #11004

Open ferdymercury opened 2 years ago

ferdymercury commented 2 years ago

Describe the bug

I see one small issue when saving a 600x600 TCanvas window as png.

I get:

Expected behavior

No pixel difference is found between running batch or not. Right now there is an offset of 1 up to 2 pixels.

To Reproduce

void test() {
    TCanvas *c = new TCanvas("c1","c1",600,600);
    c->SaveAs("c1.png");
}
//root -l test.C -b -q
//root -l test.C -q

Setup

   ------------------------------------------------------------------
  | Welcome to ROOT 6.27/01                        https://root.cern |
  | (c) 1995-2021, The ROOT Team; conception: R. Brun, F. Rademakers |
  | Built for linuxx8664gcc on Jan 11 2022, 18:39:08                 |
  | From heads/master@v6-25-01-2870-gdac9b6398d                      |
  | With c++ (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0                     |
  | Try '.help', '.demo', '.license', '.credits', '.quit'/'.q'       |
   ------------------------------------------------------------------
couet commented 1 year ago

@ferdymercury : I do not think there is a lot we can do. The png file is not a screen copy and some rounding issues in completely different code might be possible (one or 2 pixels). I propose to close this issue.

ferdymercury commented 1 year ago

In my other computer, I get now a difference of 4 pixels using latest ROOT master. So it seems to me that this is not a small rounding error.

Could it be just an outdated calibration from many years ago? See this https://github.com/root-project/root/commit/c3de92f4d1ad4372febc81955a877345d4de3a9c

Check out this code below. It seems there is always the same offset on my current computer. Whenever I run in batch mode, I get 4 pixels difference wrt batch mode. My hypothesis is that this could solved if those numbers are adapted, potentially depending on Windows / Mac with an if. It could be even added as a roottest.

#include <TCanvas.h>
#include <TROOT.h>
#include <iostream>
void test_pixels() {
    for(Int_t w=500;w<=900;w++) {
        for(Int_t h=500;h<=600;h++) {
            TCanvas *c = new TCanvas("c1","c1",w,h);
            Int_t diffw = w-c->GetWw();
            Int_t diffh = h-c->GetWh();
            if(!gROOT->IsBatch()) {
                if (diffw !=2 || diffh !=24)
                    std::cout << diffw << " " << diffh << " " << w << " " << h << std::endl;
            }
            else {
                if (diffw !=4 || diffh !=28)
                    std::cout << diffw << " " << diffh << " " << w << " " << h << std::endl;
            }
            delete c;
        }
    }
}
couet commented 1 year ago

ok, I'll look again ... Why do you need this endless loop to show the problem ? In this last example there no png generation involved as in the original question... are you pointing an other issue ?

ferdymercury commented 1 year ago

Sorry Olivier, I missed your reply.

The loop was just to see if the offset of 4 pixels appeared always, or only if the canvas was big enough. Or also to see if the offset was proportional to canvas size or not.

I am not creating PNGs in the snippet above, but rather looking at the graphics are size without borders, which is what the PNG should become later. I took the logic out of the ROOT documentation:

 To define precisely the graphics area size of a canvas in the interactive mode, the following four lines of code should be used:
{
   Double_t w = 600;
   Double_t h = 600;
   auto c = new TCanvas("c", "c", w, h);
   c->SetWindowSize(w + (w - c->GetWw()), h + (h - c->GetWh()));
}

and in the batch mode simply do:
c->SetCanvasSize(w,h);
couet commented 1 year ago

I am not sure we need to spend to much time on this issue as the new default grahics will be soon the one in the Web Browser. In the case the size of the canvas is completly different. There is a new PR on the way. In the master only for the time being.

dpiparo commented 10 months ago

I cannot understand from the conversation: is the issue fixed now?

ferdymercury commented 10 months ago

No, it's not fixed, the script reproducing it is https://github.com/root-project/root/issues/11004#issuecomment-1471869803

Though low priority ...