Opened 10 months ago

Closed 10 months ago

Last modified 10 months ago

#18530 closed defect (fixed)

Problem using Cyan color

Reported by: larrybiehl Owned by: Vadim Zeitlin <vadim@…>
Priority: normal Milestone:
Component: wxOSX Version: 3.1.2
Keywords: Cc: biehl@…
Blocked By: Blocking:
Patch: no

Description (last modified by vadz)

I have found that wxWidgets will crash if I try to use the wxCYAN color. It looks like the issue starts with how cyan is defines in GetColour in gdicmn.cpp in the core group of routines.

        switch (item)
        {
        case COLOUR_BLACK:
            colour = new wxColour(0, 0, 0);
            break;
        case COLOUR_BLUE:
            colour = new wxColour(0, 0, 255);
            break;
        case COLOUR_CYAN:
            colour = new wxColour(wxT("CYAN"));
            break;
        case COLOUR_GREEN:
            colour = new wxColour(0, 255, 0);
            break;
        case COLOUR_YELLOW:
            colour = new wxColour(255, 255, 0);
            break;
        case COLOUR_LIGHTGREY:
            colour = new wxColour(wxT("LIGHT GREY"));
            break;
        case COLOUR_RED:
            colour = new wxColour(255, 0, 0);
            break;
        case COLOUR_WHITE:
            colour = new wxColour(255, 255, 255);
            break;
        default:
            wxFAIL;
        }

Note that in the case cyan is defined as: colour = new wxColour(wxT("CYAN")); most of the other colors are defined as rgb. If I change the line to:

            colour = new wxColour(0, 255, 255);

all works okay. Why is cyan defined differently? Should the line be changed to as I have done above?

  • Larry Biehl

Attachments (1)

minimal_cyan.cpp download (14.5 KB) - added by larrybiehl 10 months ago.
File which causes a crash for me when run on MacOS.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 10 months ago by oneeyeman

  • Status changed from new to infoneeded_new

Hi,
What do I need to do to see a crash?
Can you submit a patch to a minimal or drawing sample and explain how can I see it?

Also, what OSX version are you testing it under? And what is the minimal OSX deployment version you set when you configured wxWidgets?

TIA!

comment:2 Changed 10 months ago by larrybiehl

  • Cc biehl@… added
  • Status changed from infoneeded_new to new

I am using OSX 10.13.6. The minimum version of the app (MultiSpec) is 10.7.

I am using Xcode 9.2.

It looks like the crash itself is in wxWidgets when trying to print a message indicating that the selected color is not valid. But I am not sure on this.

I will try to put together a small test case for you.

Any reason why CYAN is defined differently than the other colors?

  • Larry Biehl

Changed 10 months ago by larrybiehl

File which causes a crash for me when run on MacOS.

comment:3 Changed 10 months ago by larrybiehl

  • Cc biehl@… removed

I have created a file based on minimal.cpp (called minimal_cyan.cpp) which duplicates the crash I am seeing. It is attached to this ticket.

In doing this though, I have determined why the cyan color when defined by 'new wxColour(wxT("CYAN"));' does not work. I am setting a global gCyanPen to this color at startup before wxTheColourDataBase is defined. When gCyanPen is used later, wxWidgets tries to present a message indicating that the color is invalid when then causes the crash.

I have modified my code such that defining the cyan color works correctly without have to change the wxWidgets gdicmn.cpp file. And therefore I do not have the crash. I will leave it to you whether to determine whether something needs to be changed so that a crash does not occur. Maybe I have not set something correctly for this to happen.

Let me know if there are questions.

  • Larry Biehl

comment:4 Changed 10 months ago by oneeyeman

Hi,
2OP:
When working with wxWidgets it is strongly not advisable to use any global variable/object based on the wxWidgets classes (other than wxApp-derived one).

I don't know if it is documented somewhere, but it probably should be.

Vadim,
Should this be closed as invalid?

comment:5 Changed 10 months ago by larrybiehl

  • Cc biehl@… added

I will leave it up to you relative to resources you have. The crash will occur if for whatever reason the color given to SetPen is invalid at least for MacOS. Message was trying to be presented to user which appears to have caused the crash. It just so happens that the way I set wxPen caused the color to not be valid. Actually it was never set; it was NULL. Other ways can also probably create an invalid color which will then cause a crash.

The cause behind creating the invalid color I had is invalid.

FYI: The app I am using wxWidgets for is at: https://engineering.purdue.edu/~biehl/MultiSpec/ (the MacOS version and the Online version).

comment:6 Changed 10 months ago by vadz

  • Description modified (diff)

It looks like we could indeed use RGB values here, it's hard to see what do we gain from using named lookup. Strangely enough, we used to use names for all the colours until this was changed for most, but not all, of them in f516d986371b7643efda569d64ae19e75d221411 (commit is authored by me, but it was a patch I applied, so I really have no idea why wasn't this done for all colours or done at all).

OTOH it's true that most wx objects can't be created before the library is initialized, which definitely covers the globals, so it's still a bad idea to do it. But I can change cyan and light grey definitions nevertheless.

comment:7 Changed 10 months ago by Vadim Zeitlin <vadim@…>

  • Owner set to Vadim Zeitlin <vadim@…>
  • Resolution set to fixed
  • Status changed from new to closed

In 29be54d7/git-wxWidgets:

Define all stock colours using their fixed RGB values

For some reason, names were used for cyan and light grey, even though
all the other ones used RGB values since refactoring the stock objects
to be created on demand in f516d986371b7643efda569d64ae19e75d221411.

Use RGB values for these two as well now.

Closes #18530.

comment:8 Changed 10 months ago by Vadim Zeitlin <vadim@…>

In 29be54d7/git-wxWidgets:

Define all stock colours using their fixed RGB values

For some reason, names were used for cyan and light grey, even though
all the other ones used RGB values since refactoring the stock objects
to be created on demand in f516d986371b7643efda569d64ae19e75d221411.

Use RGB values for these two as well now.

Closes #18530.

Note: See TracTickets for help on using tickets.