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

CtrlrDocumentPanel::activeDocumentChanged() throws error on findColour(): Workaround: add IF for colourID == 16799763/5 #11

Open
unityconstruct opened this issue Mar 7, 2024 · 5 comments

Comments

@unityconstruct
Copy link

unityconstruct commented Mar 7, 2024

Running in debug mode and found the exceptions thrown for those two colors a nuisance, so added IF statement for the colourIDs specifically.
Ultimate fix might be choosing a colour from the JUCE color list... I think they are HTML(websafe) colours.
Might be able to just add them to the JUCE h file, but I didn't want to go mucking around in a bunch of places and diverge off your [master] branch.

Workaround

    if (index >= 0)
        return colours[index].colour;
    **if (colourID == 16799763 || colourID == 16799765)
        return Colours::black;**

    jassertfalse;
    return Colours::black;

Steps to Reproduce

  1. Start in Debug
  2. File>New Panel
// CtrlrManager::addPanel
void CtrlrManager::addPanel (CtrlrPanelEditor *panelToAdd)
{
	ctrlrDocumentPanel->addDocument ((Component *)panelToAdd, Colours::lightgrey, true);
}

// memory
    _NODISCARD pointer operator->() const noexcept {
        return _Mypair._Myval2;
    }

image

//juce_MultiDocumentPanel.cpp

bool MultiDocumentPanel::addDocument (Component* const component,
                                      Colour docColour,
                                      const bool deleteWhenRemoved)
{
    // If you try passing a full DocumentWindow or ResizableWindow in here, you'll end up
    // with a frame-within-a-frame! Just pass in the bare content component.
    jassert (dynamic_cast<ResizableWindow*> (component) == nullptr);

    if (component == nullptr || (maximumNumDocuments > 0 && components.size() >= maximumNumDocuments))
        return false;

    components.add (component);
    component->getProperties().set ("mdiDocumentDelete_", deleteWhenRemoved);
    component->getProperties().set ("mdiDocumentBkg_", (int) docColour.getARGB());

// Autos: 
// juce::Colour::getARGB returned | 4292072403 | unsigned int


// CtrlrDocumentPanel::activeDocumentChanged
void CtrlrDocumentPanel::activeDocumentChanged()
{
	CtrlrEditor *ed = dynamic_cast <CtrlrEditor*> (getParentComponent());
	if (ed)
    {
		ed->activeCtrlrChanged();
        setBackgroundColour(Component::findColour(DocumentWindow::backgroundColourId));
    }
    if (getCurrentTabbedComponent()) {
        getCurrentTabbedComponent()->setTabBarDepth(owner.getProperty(Ids::ctrlrTabBarDepth));
        getCurrentTabbedComponent()->getTabbedButtonBar().setColour(TabbedButtonBar::tabTextColourId, **findColour(TabbedButtonBar::tabTextColourId)); // Not working
        getCurrentTabbedComponent()->getTabbedButtonBar().setColour(TabbedButtonBar::frontTextColourId, findColour(TabbedButtonBar::frontTextColourId)); // Not working**
    }
    setBackgroundColour(Colours::lightgrey); // Sets background colour behind main window by default on grey to please everyone :)
}

//Colour LookAndFeel::findColour 
Colour LookAndFeel::findColour (int colourID) const noexcept
{
    const ColourSetting c = { colourID, Colour() };
    auto index = colours.indexOf (c);

    if (index >= 0)
        return colours[index].colour;
    **if (colourID == 16799763 || colourID == 16799765)
        return Colours::black;**

    jassertfalse;
    return Colours::black;
}
@damiensellier
Copy link
Owner

Hi UC,

i have some trouble understanding what's the problem.
how did you manage to get those colour index 16799763 & 16799765 errors ?
Is it docColour having a problem in the declaration?
https://docs.juce.com/master/classMultiDocumentPanel.html#a92a4b424c5d8eee7fb859e5a2e71bdad

backgroundColour the background colour to use to fill the component's window or tab

What is the result on the GUI? wrong colours? the app crashes?

It's always better to avoid injecting the workaround statement in the JUCE core module juce_lookandfeel.cpp.
I'll try to find a way to implement that in the ctrlr files instead if I understand exactly what is going wrong with those colours.

Thanks

Damien

@unityconstruct
Copy link
Author

unityconstruct commented May 7, 2024

  • Been a while, but best I recall off-hand is on W10 host starting CtrlrX in DEBUG will open the app up without issue.
  • Opening a panel kicks off some initialization routines with respect to color and it seemed that it was checking colors against a pallet & if NOT in the color pallet, it would init the color to black.
  • Not a problem in itself, but it would do this many times - each raising a breakpoint - so when loading a panel, you'd have to ACK the breakpoint tons of times before getting an idle state where one could interact with the panel.
  • If starting CtrlrX in DEBUG and opening a few panels doesn't induce the issue, let me know & I can do OBS screencapture.
  • If you're on Mac & not seeing it, this might be Windows-only, but given the mundane nature of color-pallet lookups seems it would be OS agnostic ( unless the LookNFeel is OS-aware and uses different color-pallets perhaps ).
  • there was no visible impact on the panel when defaulting any of the colors to black & I wasn't readily able to identify where the COLORID being passed in was originating from.

@unityconstruct
Copy link
Author

OK dusted off my VS box... (used the proteus panel in case there was something specific to the panel I'm working on.)

  • Start debug session
    image
  • CtrlrX starts fine with no panels autoloaded
  • open the proteus panel & breakpoint hit before any UI elements loaded
  • one can hit continue repetitively to ACK each one, but there were quite a few
  • I couldn't find a way to ignore the jassert breakpoints in VStudio & assuming since its a java hook in JUCE, there's no real way to ignore it in VStudio, thus I opted to simply add hard-coded handling ( which I agree isn't the best approach ) for those specific color ids.
  • The jassert gets hit more than 50x, but seems only for id=16777859 and id=16777860 ( NOTE THESE ARE DIFFERENT THAN ORIGINAL POST, SO the workaround for my panel clearly isn't a universal fix )
    image
  • If you ACK all the jasserts, the panel loads fine if I recall.

CtrlrX UI

  • File > Open
  • select panel
  • click OK
  • BP hit before panel UI loads
  • would note that it is also before the CtrlrEditorPanel is loaded
    image

VStudio

juce_LookAndFeel.cpp line85

A breakpoint instruction (__debugbreak() statement or a similar call) was executed in CtrlrX.exe.
image

Test Panel

Emu-Proteus-2_1_0.zip

System Specs

image

@unityconstruct
Copy link
Author

Context on the logic in the method

  • if statement
    • if colorid match is found, return it ( exit method )
    • else jassertfalse; & return Colours::black;
      image

@damiensellier
Copy link
Owner

Thanks for all the details, I'm not home for a while but I'll check the process once back.

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

2 participants