Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macOS regression with glwindow, glsubwindow and tile. #968

Closed
ggarra13 opened this issue May 6, 2024 · 14 comments
Closed

macOS regression with glwindow, glsubwindow and tile. #968

ggarra13 opened this issue May 6, 2024 · 14 comments
Assignees
Labels
Platform: macOS platform specific (macOS)

Comments

@ggarra13
Copy link
Contributor

ggarra13 commented May 6, 2024

Describe the bug
There's been a regression on macOS with OpenGL windows and Tile, where the tile is not respected.

To Reproduce
N/A yet.

I did not do a full bisect yet, but:

7ec6f96 # okay
eeed395 # broken

Expected behavior
The behavior should be consistant between both tags (it should be like in okay).

Screenshots
Notice the topbar with "Color", exposure, etc. Also note the separator (dragger) between the timeline and the view window.
macos_ok

See how in this version the main viewport window is bigger. It partially overlaps the top bar and the separator betweeen the timeline and view window is gone. Also, the dragger would show inside the timeline (it would change the icon only there) and crashes happen when showing thumbnail previews parented to the timeline.
macos_broken

FLTK Version
Please complete the following information and delete non-applicable lines:

  • Version: 1.4.0
  • See tags listed above.

FLTK Configure / Build Options
N/A. Same with both.

Operating System / Platform:
Please be as precise as possible, e.g. "Linux: Ubuntu 20.04"

  • OS: macOS
  • OS Version: Sonoma 14
  • Processor: Intel (but it seems to also happen on M1 -- cannot test as I don't have one).
@MatthiasWM
Copy link
Contributor

I was afraid that there is a use case where my easy fix would not work. It seems to obvious. Do you think you can generate a small program that shows the behavior? That would be very helpful.

@ggarra13
Copy link
Contributor Author

ggarra13 commented May 6, 2024

Steps to reproduce. I thought it was @ManoloFLTK (sorry @MatthiasWM) It is long, but trust me, it is easy:

  1. Clone the repository.

git clone https://github.com/ggarra13/mrv2.git --branch manolo_macos

  1. Create a bin directory in your $HOME dir.

mkdir -p $HOME/bin

  1. Go to the root of the repository.
    Keep this terminal at the root of the repo so you can run quick
    iterations.

cd mrv2

  1. Open the FLTK Build script.

nano cmake/Modules/BuildFLTK.cmake

  1. Change the variables to point to your own repository and master
    (or whatever tag you want to checkout first). We do this so you
    can work on this without having to recompile the whole project.

#set(FLTK_GIT_REPOSITORY "https://github.com/fltk/fltk.git")
set(FLTK_GIT_REPOSITORY "git@github.com:fltk/fltk.git")
set(FLTK_GIT_TAG master)

  1. Compile my program as debug (on my MacBook Pro Intel it takes 7 mins).

bin/runme_manolo.sh debug -y

This places the program and libs inside:

BUILD-Darwin-arm64/Debug/install/bin
BUILD-Darwin-arm64/Debug/install/include/FL
BUILD-Darwin-arm64/Debug/install/lib
  1. Download this test movie. You can use your own. I think you only have to make sure it covers the whole screen.
    https://mega.nz/file/XXghgIQK#ZTBlrNB4BgpXE3U44_iVPNLchDP7XRwbb2Nd4b-KLjY

  2. Run the program (it will have placed a symlink in $HOME/bin):

    mrv2-dbg elemental-trailer-1_h720p.mov

  3. Open a new terminal window. Make testing changes to FLTK.

   cd <mrv2_repository_dir>
   cd BUILD-Darwin-arm64/Debug (or amd64)
   cd FLTK-prefix/src/FLTK/src
   git switch -C macos_fix
   emacs src/Fl_Gl_Window.cxx (or whatever...)
   # do git checkout .. or git switch .. or whatever in this this
  1. From the other terminal, which we left at the root of the repository.
    Recompile FLTK testing changes and my program (it takes 30 seconds):

runmet.sh debug

  1. Redo step 8, 9 and 10 until you are happy with FLTK code.

  2. Commit your changes:

   cd <mrv2_repository_dir>
   cd BUILD-Darwin-arm64/Debug (or amd64)
   cd FLTK-prefix/src/FLTK/src
   git commit -a
  1. Push it into FLTK's main repository (ie. that's why I had you change
    the default FLTK_GIT_REPOSITORY).

git push

@ggarra13
Copy link
Contributor Author

ggarra13 commented May 6, 2024

I think the issue happens when the window is refit to cover the whole screen work area (so you might need to use a 4K movie for testing).

@ManoloFLTK
Copy link
Contributor

@ggarra13 The procedure you propose is far too complex. Sorry. The issue you mention apparently occurs without any GL scene running. I would expect a simple program with a couple of subwindows and Fl_Tile objects mimicking your app's main window to be enough to trigger the bug and help FLTK developers correct it.

@ggarra13
Copy link
Contributor Author

ggarra13 commented May 7, 2024

@MatthiasWM @ManoloFLTK

Ok. I was able to simplify it and reproduce in a simple example.

Just call:

runme.sh

macos_bug.zip

@ManoloFLTK
Copy link
Contributor

@ggarra13 OK. I've downloaded macos_bug.zip and see I can build and run the small program inside on macOS. What is the bug therein?

@ggarra13
Copy link
Contributor Author

ggarra13 commented May 8, 2024

First, you can easily compile the main.cpp file with fltk-config --use-gl.

The bug seems to get triggered on a resize on a timeout when it fills the full screen.
You may need to adjust the renderSize variable if your screen is big.

Here's what I see on my Macbook Pro with tags I posted before:

OK:
macos_ok

BAD (notice how the "Input Color Space" bar is covered by the gl window):
macos_bad

@ManoloFLTK
Copy link
Contributor

@ggarra13 The problem is in your calculations to position and size your toplevel window in the screen that are plain wrong. Use this modified code for your function cb_Resize() and you'll see that the bug disappears. You'll see also that the window is correctly positioned both when the dock is at bottom or at left (or right) of the display.

Your errors are mainly to forget that
Fl::screen_work_area(minx, miny, maxW, maxH, screen);
compute two positions (minx and miny) and two sizes (maxW, maxH), and to compare positions to sizes.

static void cb_Resize(Fl_Widget* o, void* d)
{
    std::cerr << "resize called" << std::endl;
    RenderSize renderSize;
  renderSize.w =  1950; // big values for big and also for small screens
  renderSize.h = 900;
    Fl_Double_Window* mw = uiMain;
    int screen = mw->screen_num();
    //float scale = Fl::screen_scale(screen);

    int W = renderSize.w;
    int H = renderSize.h;

    int minx, miny, maxW, maxH, posX, posY;
    Fl::screen_work_area(minx, miny, maxW, maxH, screen);

    posX = mw->x();
    posY = mw->y();

    int decW = mw->decorated_w();
    int decH = mw->decorated_h();

    int dW = decW - mw->w();
    int dH = decH - mw->h();

    maxW -= dW;
    //maxH -= dH;
    posX += dW / 2;
#ifdef _WIN32
    miny += dH - dW / 2;
#endif

    // Take into account the different UI bars
    if (uiMenuGroup->visible())
        H += uiMenuGroup->h();

    if (uiTopBar->visible())
        H += uiTopBar->h();

    if (uiPixelBar->visible())
        H += uiPixelBar->h();

    if (uiBottomBar->visible())
    {
        H += uiBottomBar->h();
    }

    if (uiStatusGroup->visible())
        H += uiStatusGroup->h();

    if (uiToolsGroup->visible())
        W += uiToolsGroup->w();

    if (uiDockGroup->visible())
        W += uiDockGroup->w();

    //maxW = (int)(maxW / scale); // bad: maxW accounts for scale already
    if (W < 690)
    {
        W = 690;
    }
    /*else if (W > maxW) // bad: done in final sanity checks
    {
        W = maxW;
    }*/

    //maxH = (int)(maxH / scale); // bad: maxH accounts for scale already
    if (H < 602)
    {
        H = 602;
    }
    /*else if (H > maxH) // bad: done in final sanity checks
    {
        H = maxH;
    }*/

    if (uiBottomBar->visible())
    {
        H += uiBottomBar->h();
    }

    //
    // Final sanity checks.
    //
    /*if (W > maxW)
        W = maxW;
    if (H > maxH)
        H = maxH;*/

    /*if (posX + W > maxW) // bad: left of > is a position, right of > is a size!
        posX = minx;*/
  if (posX + W > minx+maxW) {
    posX = minx;
    W = minx+maxW - posX;
  }
  
    /*if (posY + H > maxH) // bad: left of > is a position, right of > is a size!
        posY = miny;*/
  if (posY + H > miny+maxH) {
    posY = miny + dH;
    H = miny+maxH - posY;
  }

  std::cerr << "mw resize=" << posX << ", " << posY
              << " " << W << "x" << H << std::endl;
    mw->resize(posX, posY, W, H);

    std::cerr << "1 uiView->y()=" << uiView->y() << std::endl;
    uiRegion->layout();
    std::cerr << "2 uiView->y()=" << uiView->y() << std::endl;

    // Set_mode_cb();
}

@ManoloFLTK ManoloFLTK self-assigned this May 8, 2024
@ManoloFLTK ManoloFLTK added the Platform: macOS platform specific (macOS) label May 8, 2024
@ManoloFLTK
Copy link
Contributor

@ggarra13 It turns out that commit 3f91c8b had a remaining problem revealed by this issue and fixed by 324fcfc. Do you see your issue fixed with the modified cb_Resize() mentionned above and 324fcfc? If yes, please, close this issue.

@ggarra13
Copy link
Contributor Author

ggarra13 commented May 8, 2024

@ManoloFLTK Yes, I see the issue fixed now. Clossing the issue.

@ggarra13 ggarra13 closed this as completed May 8, 2024
@ManoloFLTK
Copy link
Contributor

@ggarra13 It occurs now to me that the new in 1.4 member function Fl_Window::maximize() is perfect for you. It can resize your window so it covers all the display without requiring any coordinate and size computation.

@ggarra13
Copy link
Contributor Author

ggarra13 commented May 9, 2024

@ManoloFLTK Thanks, Manolo. But in my real application, I have to do more than that. I allow position and sizing the window to any values (so the viewer sticks in one position or to a certain size in the Desktop). That's why I keep posX and posY and have to do all sort of sanity checks at the end.

@ggarra13 ggarra13 reopened this May 12, 2024
@ggarra13
Copy link
Contributor Author

I found out the issue it is still not fixed correctly on macOS.

I am attaching a file with your original fix (which I think was wrong). And with my actual code that still shows the issue. See the comments in the code. I am using latest FLTK master.

main.cpp.txt

To compile use fltk-config --compile main.cpp --use-gl

@ggarra13
Copy link
Contributor Author

Never mind. It was a simple math issue. Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: macOS platform specific (macOS)
Projects
None yet
Development

No branches or pull requests

3 participants