cpc / openasip

Open Application-Specific Instruction Set processor tools (OpenASIP)
http://openasip.org
Other
140 stars 42 forks source link

Prode: fix operand usage dialog box for dark mode #206

Closed TopiLeppanen closed 1 year ago

TopiLeppanen commented 1 year ago

In Ubuntu 20's dark mode the FU's operand usage dialog (the one with R's and W's) was white-on-white. This change makes it so in light-mode it's black-on-white, and in dark-mode white-on-grey.

Maybe somebody else should test this on different system? I tested it with ubuntu 20.04 Gnome's light and dark mode

sarahec commented 1 year ago

I can verify this tomorrow. (Was installing a new motherboard in my computer and let the magic smoke out of the processor, with a replacement coming tomorrow.)

sarahec commented 1 year ago

I can see it working the same way on a different system (in this case, using the Qt rendering layer for wxWindows).

I think there may be a better solution than explicitly selecting colors, but I need to do a bit more experimentation .

sarahec commented 1 year ago

See what happens if you remove the background colors on lines 149-152 and line 160:

    // resourceGrid_->SetDefaultCellBackgroundColour(
    //     wxSystemSettings::GetColour(wxSYS_COLOUR_BTNFACE));
    // resourceGrid_->SetSelectionBackground(ProDeConstants::WHITE_COLOR);
    // resourceGrid_->SetSelectionForeground(ProDeConstants::BLACK_COLOR);

and (160)

    // usageGrid_->SetDefaultCellBackgroundColour(ProDeConstants::WHITE_COLOR);

It works correctly in both light and dark mode.

sarahec commented 1 year ago

@TopiLeppanen which resolution would you like? Merge the patch as is, or rework to remove the specified lines and use the theme defaults?

TopiLeppanen commented 1 year ago

Hi, Thanks a lot for investigating this, and sorry I didn't get back to you earlier. To me it feels more natural to use the theme defaults instead of overriding defaults if there's no good reason to.

I tested your suggestion of removing the overrides and it seems to work well for me as well, so I decided to apply it for this PR.

pjaaskel commented 1 year ago

Can you update this to CHANGES?

TopiLeppanen commented 1 year ago

Updated CHANGES

I started a new section for 2.1