Skip to content

Fix rev graph

Yue Lin Ho requested to merge YueLinHo/tortoisegit:fix_rev_graph into master

The first commit resolves this that the icons are not showed:
rev_graph_issue

with this commit:
rev_graph

But, the error shows up while running in VS2017 debug mode:
ASSERT_error

@lznuaa tried to fix the assertion failed issue in 2012, see b3a25d89
Somehow, it can be ignored by just pushing the Ignore button or "I" key.
So, I guess that why TortoiseSVN does not fix it.

And, I traced some...

Here is the root cause: AfxLoadSysColorBitmap() only supports up to 8-bit bitmap, but the bitmap we use is 24-bit.
root_cause

then AfxLoadSysColorBitmap() return NULL:
return_null

and assert failed here (in CToolBar::AddReplaceBitmap()):
verify

Also, I noticed that there is a "tweaking" in CRevisionGraphDlg::InitializeToolbar() (see this line )

So, the coming 2 commits fix the assertion failed issue by copying CToolBar::LoadToolBar() code from bartool.cpp, then fitting the "tweaking" into the new CRevGraphToolBar::LoadToolBar() member function.

NOTE:

  1. AfxLoadSysColorBitmap() is used in CToolBar::LoadBitmap() and CToolBar::OnSysColorChange().
    So, the icons might be not reloaded after the system color is changed.
    (the member m_hInstImageWell and m_hRsrcImageWell is always NULL.)
  2. CToolBar::LoadBitmap() is only used in CToolBar::LoadToolBar(), and not a virtual function.
    So, can not override it to fix up.
  3. CToolBar::LoadToolBar() is not a virtual function.
    So, the CRevGraphToolBar::LoadToolBar() is just a new member function for doing the almost same thing the CToolBar::LoadToolBar() does:
    (1) Set Buttons
    (2) Load Bitmap

cf. CToolbar class defined in c:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.13.26128\atlmfc\include\afxext.h

Edited by Yue Lin Ho

Merge request reports