eeschema debug build hits assert when adding component

[ronnie]

This only to affects debug builds.

Steps for reproducing:
* Open eeschema with an empty schematic,
* Ensure Preferences > Colors > Body Background color has an uniform color (as in r = g = b) e.g. white (r = g = b = 255) or grayish
* Press A to add a component
* Select any part to insert
* Confirm dialog with OK

Assert is hit at
eeschema: /home/ronnie/projects/kicad-source-mirror/include/gal/color4d.h:61: KIGFX::COLOR4D::COLOR4D(double, double, double, double): Assertion `b >= 0.0 && b <= 1.0' failed.

Affected version info is ()

Application: eeschema
Version: (6.0.0-rc1-dev-831-gcbf74d6), debug build
Libraries:
    wxWidgets 3.0.2
Platform: Linux 4.4.0-137-generic x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.2 (wchar_t,wx containers,compatible with 2.8) GTK+ 2.24
    Boost: 1.58.0
    Compiler: GCC 5.4.0 with C++ ABI 1009

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=OFF
    KICAD_SCRIPTING=OFF
    KICAD_SCRIPTING_MODULES=OFF
    KICAD_SCRIPTING_WXPYTHON=OFF
    KICAD_SCRIPTING_ACTION_MENU=OFF
    BUILD_GITHUB_PLUGIN=OFF
    KICAD_USE_OCE=OFF
    KICAD_USE_OCC=OFF
    KICAD_SPICE=OFF

Stack Trace from CLion:
raise 0x00007fa52727e428
abort 0x00007fa52728002a
<unknown> 0x00007fa527276bd7
__assert_fail 0x00007fa527276c82
KIGFX::COLOR4D::COLOR4D color4d.h:61
KIGFX::COLOR4D::WithAlpha color4d.h:241
KIGFX::SCH_PAINTER::defaultColors sch_painter.cpp:306
KIGFX::SCH_PAINTER::draw sch_painter.cpp:285
KIGFX::SCH_PAINTER::Draw sch_painter.cpp:153
KIGFX::SCH_PAINTER::<lambda(LIB_ITEM&, bool)>::operator()(LIB_ITEM &, bool) const sch_painter.cpp:240
KIGFX::SCH_PAINTER::draw sch_painter.cpp:250
KIGFX::SCH_PAINTER::draw sch_painter.cpp:1068
KIGFX::SCH_PAINTER::Draw sch_painter.cpp:160
KIGFX::VIEW_GROUP::ViewDraw view_group.cpp:183
KIGFX::VIEW::draw view.cpp:1029
KIGFX::VIEW::drawItem::operator() view.cpp:960
RTree<KIGFX::VIEW_ITEM*, int, 2, double, 8, 4>::Search<KIGFX::VIEW::drawItem> rtree.h:499
RTree<KIGFX::VIEW_ITEM*, int, 2, double, 8, 4>::Search<KIGFX::VIEW::drawItem> rtree.h:154
KIGFX::VIEW_RTREE::Query<KIGFX::VIEW::drawItem> view_rtree.h:85
KIGFX::VIEW::redrawRect view.cpp:999
KIGFX::VIEW::Redraw view.cpp:1135
EDA_DRAW_PANEL_GAL::onPaint draw_panel_gal.cpp:208
SCH_DRAW_PANEL::onPaint sch_draw_panel.cpp:681
wxAppConsoleBase::CallEventHandler(wxEvtHandler*, wxEventFunctor&, wxEvent&) const 0x00007fa5281311fe
wxEvtHandler::ProcessEventIfMatchesId(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) 0x00007fa5282b76e7
wxEvtHandler::SearchDynamicEventTable(wxEvent&) 0x00007fa5282b7ace
wxEvtHandler::TryHereOnly(wxEvent&) 0x00007fa5282b7b5f
wxEvtHandler::ProcessEventLocally(wxEvent&) 0x00007fa5282b7c13
wxEvtHandler::ProcessEvent(wxEvent&) 0x00007fa5282b7c75
wxEvtHandler::ProcessPendingEvents() 0x00007fa5282b86fa
wxAppConsoleBase::ProcessPendingEvents() 0x00007fa528133317
wxApp::DoIdle() 0x00007fa5289f5ac2
<unknown> 0x00007fa5289f5bb3
g_main_context_dispatch 0x00007fa525aa004a
<unknown> 0x00007fa525aa03f0
g_main_loop_run 0x00007fa525aa0712
gtk_main 0x00007fa526d2e697
wxGUIEventLoop::DoRun() 0x00007fa528a15ed5
wxEventLoopBase::Run() 0x00007fa528173353
wxAppConsoleBase::MainLoop() 0x00007fa528138666
APP_SINGLE_TOP::OnRun single_top.cpp:186
wxEntry(int&, wchar_t**) 0x00007fa5281c4f70
main single_top.cpp:260
__libc_start_main 0x00007fa527269830
_start 0x0000000000437fb9

[ronnie (ronnie81)]

That bugfix might be just a symptomatic fix and not addressing the root cause.

Usage of function
    ToHSV(double &, double &, double &, bool) const : void
Found usages (8 usages found)
    Production (8 usages found)
        kicad-source-mirror (8 usages found)
            common/dialogs (4 usages found)
                dialog_color_picker.cpp (4 usages found)
                    buttColorClick (1 usage found)
                        506 m_newColor4D.ToHSV( m_hue, m_sat, m_val, true );
                    DIALOG_COLOR_PICKER (1 usage found)
                        35 m_newColor4D.ToHSV( m_hue, m_sat, m_val, true );
                    onRGBMouseDrag (1 usage found)
                        606 m_newColor4D.ToHSV( m_hue, m_sat, m_val, true );
                    SetEditVals (1 usage found)
                        457 m_newColor4D.ToHSV( m_hue, m_sat, m_val, true );
            common/gal (4 usages found)
                color4d.cpp (4 usages found)
                    GetNearestLegacyColor (3 usages found)
                        179 aColor.ToHSV( h, s, v );
                        200 candidate4d.ToHSV( ch, cs, cv );
                        229 candidate4d.ToHSV( ch, cs, cv );
                    Saturate (1 usage found)
                        403 ToHSV( h, s, v, true );

[Jeff Young (jeyjey)]

Hi Ronnie,

Thanks for the patch. I think it would be safer, though, to flip the default value of aAlwaysDefineHue and then change the 3 calls in COLOR4D::GetNearestLegacyColor() to pass false.

That way future users of ToHSV() won't fall into the same trap.

Cheers,
Jeff.

[KiCad Janitor (kicad-janitor)]

Fixed in revision ab1f01613259fac84843d1b8917f364d1f619993
https://git.launchpad.net/kicad/patch/?id=ab1f01613259fac84843d1b8917f364d1f619993

[ronnie (ronnie81)]

Hi Jeff,

thanks for already merging that patch. I follow your advise to change the default parameter and revise all call to ToHSV(). Patch is attached.

On the side of investigating this I found a comment in include/colors.h that the legacy style color.h/cpp is going away anyways. That's now, after all the work invested in gal & eeschema dev, isn't it?

[ronnie (ronnie81)]

PS: Actually thanks to Thomas for merging ;) But Jeff thanks for the feedback

[Jeff Young (jeyjey)]

Only the Eeschema legacy canvas is going away in 5.1. The PcbNew legacy canvas will likely be retired in 6.0.