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

Resize API and Aspect Ratio interact improperly; current implementation is rather crude #56

Open
baconpaul opened this issue Jun 21, 2022 · 5 comments

Comments

@baconpaul
Copy link
Collaborator

baconpaul commented Jun 21, 2022

Right now the CLAP resize API doesn't support constrainers properly so things like aspect ratio and the like are amiss.

I have a fix for this but it runs afoul of a BWS 43b7 problem, reported here free-audio/interop-tracker#30

Once that issue is resolved, the branch mentioned in that issue will allow us to resolve this

@baconpaul baconpaul changed the title Resize API is improperly supported Resize API is improperly implemented; ignores juce constrainers Jun 22, 2022
baconpaul added a commit to baconpaul/clap-juce-extensions that referenced this issue Jun 22, 2022
This is a first shot at making adjust size / set size work.
It naively just applies aspect ratio to with which makes corner
and bottom drags work but not side drags.

An algorithm to work for all drags without knowing the direction
of the user gesture which is stable continues to elude me, but I wanted
at least something more reasonable in teh codebase before I go on vacation.

Addresses free-audio#56
baconpaul added a commit to baconpaul/clap-juce-extensions that referenced this issue Jun 22, 2022
This is a first shot at making adjust size / set size work.
It naively just applies aspect ratio to with which makes corner
and bottom drags work but not side drags.

An algorithm to work for all drags without knowing the direction
of the user gesture which is stable continues to elude me, but I wanted
at least something more reasonable in teh codebase before I go on vacation.

Addresses free-audio#56
baconpaul added a commit to baconpaul/clap-juce-extensions that referenced this issue Jun 23, 2022
This is a first shot at making adjust size / set size work.
It naively just applies aspect ratio to with which makes corner
and bottom drags work but not side drags.

An algorithm to work for all drags without knowing the direction
of the user gesture which is stable continues to elude me, but I wanted
at least something more reasonable in teh codebase before I go on vacation.

Addresses free-audio#56
jatinchowdhury18 pushed a commit that referenced this issue Jun 23, 2022
* A first attempt at improving the adjust size approach

This is a first shot at making adjust size / set size work.
It naively just applies aspect ratio to with which makes corner
and bottom drags work but not side drags.

An algorithm to work for all drags without knowing the direction
of the user gesture which is stable continues to elude me, but I wanted
at least something more reasonable in teh codebase before I go on vacation.

Addresses #56

* Fix for pipelin

* There is no std clamp in c++14

* Respond to review comments
@baconpaul baconpaul changed the title Resize API is improperly implemented; ignores juce constrainers Resize API and Aspect Ratio interact improperly; current implementation is rather crude Jun 23, 2022
@baconpaul
Copy link
Collaborator Author

See the comment in the code and the above issue and pr for more details

@baconpaul
Copy link
Collaborator Author

OK so here's where we are with bws 4.3 b9 (and tagging in @abique here)

I think it might be impossible to write guiAdjustSize stably

If I take clap-juce-wrapper and set the mode to terminate / maximal using the undefs at the top

and then replace the adjust size with this

            // width = std::round(aspectRatio * height);
            bool adjustWidth = (1.0 * width / height > aspectRatio);

            if (adjustWidth)
            {
                width = (uint32_t)std::round(1.0 * height * aspectRatio);
            }
            else
            {
                height = (uint32_t)std::round(1.0 * width / aspectRatio);
            }

in surge I get this

clap_plugin_gui.adjust_size() isn't stable:
  (817, 569) -> (817, 559)
  (817, 559) -> (816, 559)
  !! Check you're rounding math!
libc++abi: terminating

In surge the aspect ratio is is 1.46042

So lets do the math.

817 / 569 -> 1.435852 so that is < aspect ratio. so adjust height

height -> 817 / 1.46042 = 559.4281 rounds to 559. Great

Then lets put in 817, 559

817 / 559 = 1.462153 which is > aspect ratio

so width goes to 559 * 1.46042 = 816.374 round to 816

in other words, that simple algorithm isn't stable under rounding.

This makes bitwig throw an exception and blammo.

@baconpaul
Copy link
Collaborator Author

If I add to the end of that calculation

 // never do more less than a 2px adjustment
            if (abs((int)width - (int)*w) < 2 && abs((int)height - (int)*h) < 2)
            {
                width = *w;
                height = *h;
            }

then it no longer crashes. Namely clamp my changes always.

(It then only ever shrinks with side and bottom drags but that's a separate question)

@baconpaul
Copy link
Collaborator Author

OK I got firther

If you run clap-juce-extensions from baconpaul/almost-sized then

  1. it is stable
  2. left and bottom drag work
  3. but corner drag flickers nastily

at least on a mac retina display with surge

i can pick this up in a couple of weeks but if anyone else wants to take a swing I would welcome it.

@baconpaul
Copy link
Collaborator Author

baconpaul commented Jun 24, 2022

Oh and here's the code in that branch

           auto ch = editor->getBounds().getHeight();
            auto cw = editor->getBounds().getWidth();

            auto dw = (int)width - (int)cw;
            auto dh = (int)height - (int)ch;

            bool adjustWidth = true;

            if (abs(dw) > 2 && abs(dh) <= 2)
            {
                adjustWidth = false;
            }
            else if (abs(dh) > 2 && abs(dw) <= 2)
            {
                adjustWidth = true;
            }

            if (adjustWidth)
            {
                width = (uint32_t)std::round(1.0 * height * aspectRatio);
            }
            else
            {
                height = (uint32_t)std::round(1.0 * width / aspectRatio);
            }

            // never do more less than a 2px adjustment
            if (abs((int)width - (int)*w) < 2 && abs((int)height - (int)*h) < 2)
            {
                width = *w;
                height = *h;
            }

            if (abs(dw) <= 1 && abs(dh) <= 1)
            {
                width = *w;
                height = *h;
            }

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

No branches or pull requests

1 participant