Skip to content

Commit

Permalink
in release make ReportIf() break if running under debugger; de-temple…
Browse files Browse the repository at this point in the history
…tize limitValue()
  • Loading branch information
kjk committed May 30, 2024
1 parent 0d709af commit 54cfbb5
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 36 deletions.
2 changes: 1 addition & 1 deletion src/Canvas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,7 @@ static void DrawDocument(MainWindow* win, HDC hdc, RECT* rcArea) {
}

bool renderOutOfDateCue = false;
int renderDelay = gRenderCache.Paint(hdc, bounds, dm, pageNo, pageInfo, &renderOutOfDateCue);
int renderDelay = gRenderCache->Paint(hdc, bounds, dm, pageNo, pageInfo, &renderOutOfDateCue);

if (renderDelay != 0) {
HFONT fontRightTxt = CreateSimpleFont(hdc, "MS Shell Dlg", 14);
Expand Down
2 changes: 1 addition & 1 deletion src/StressTesting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,7 @@ static void OnTimer(StressTest* st, int timerIdGot) {
// Image files are always fully rendered in WM_PAINT, so we know the page
// has already been rendered.
dm = st->win->AsFixed();
didRender = gRenderCache.Exists(dm, st->currPageNo, dm->GetRotation());
didRender = gRenderCache->Exists(dm, st->currPageNo, dm->GetRotation());
if (!didRender && dm->ShouldCacheRendering(st->currPageNo)) {
double timeInMs = TimeSinceInMs(st->currPageRenderTime);
if (timeInMs > 3.0 * 1000) {
Expand Down
24 changes: 12 additions & 12 deletions src/SumatraPDF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ static Kind kNotifZoom = "zoom";
Favorites gFavorites;

HBITMAP gBitmapReloadingCue;
RenderCache gRenderCache;
RenderCache* gRenderCache;
HCURSOR gCursorDrag;

// set after mouse shortcuts involving the Alt key (so that the menu bar isn't activated)
Expand Down Expand Up @@ -726,7 +726,7 @@ void ControllerCallbackHandler::RenderThumbnail(DisplayModel* dm, Size size, con

// TODO: this is leaking?
RenderingCallback* callback = new ThumbnailRenderingTask(saveThumbnail);
gRenderCache.Render(dm, 1, 0, zoom, pageRect, *callback);
gRenderCache->Render(dm, 1, 0, zoom, pageRect, *callback);
// cppcheck-suppress memleak
}

Expand Down Expand Up @@ -788,13 +788,13 @@ void ControllerCallbackHandler::RequestRendering(int pageNo) {
// they'll be rendered directly in DrawDocument during
// WM_PAINT on the UI thread
if (dm->ShouldCacheRendering(pageNo)) {
gRenderCache.RequestRendering(dm, pageNo);
gRenderCache->RequestRendering(dm, pageNo);
}
}

void ControllerCallbackHandler::CleanUp(DisplayModel* dm) {
gRenderCache.CancelRendering(dm);
gRenderCache.FreeForDisplayModel(dm);
gRenderCache->CancelRendering(dm);
gRenderCache->FreeForDisplayModel(dm);
}

void ControllerCallbackHandler::FocusFrame(bool always) {
Expand Down Expand Up @@ -1204,7 +1204,7 @@ static void ReplaceDocumentInCurrentTab(LoadArgs* args, DocController* ctrl, Fil
dm->SetDisplayR2L(fs ? fs->displayR2L : gGlobalPrefs->comicBookUI.cbxMangaMode);
}
if (prevCtrl && prevCtrl->AsFixed() && str::Eq(win->ctrl->GetFilePath(), prevCtrl->GetFilePath())) {
gRenderCache.KeepForDisplayModel(prevCtrl->AsFixed(), dm);
gRenderCache->KeepForDisplayModel(prevCtrl->AsFixed(), dm);
dm->CopyNavHistory(*prevCtrl->AsFixed());
}
// tell UI Automation about content change
Expand Down Expand Up @@ -2178,8 +2178,8 @@ void MainWindowRerender(MainWindow* win, bool includeNonClientArea) {
if (!dm) {
return;
}
gRenderCache.CancelRendering(dm);
gRenderCache.KeepForDisplayModel(dm, dm);
gRenderCache->CancelRendering(dm);
gRenderCache->KeepForDisplayModel(dm, dm);
if (includeNonClientArea) {
win->RedrawAllIncludingNonClient();
} else {
Expand All @@ -2206,12 +2206,12 @@ void UpdateDocumentColors() {
COLORREF text = ThemeDocumentColors(bg);
// logfa("retrieved doc colors in UpdateDocumentColors: 0x%x 0x%x\n", text, bg);

if ((text == gRenderCache.textColor) && (bg == gRenderCache.backgroundColor)) {
if ((text == gRenderCache->textColor) && (bg == gRenderCache->backgroundColor)) {
return; // colors didn't change
}

gRenderCache.textColor = text;
gRenderCache.backgroundColor = bg;
gRenderCache->textColor = text;
gRenderCache->backgroundColor = bg;
RerenderEverything();
}

Expand Down Expand Up @@ -3543,7 +3543,7 @@ static void RelayoutFrame(MainWindow* win, bool updateToolbars = true, int sideb
} else {
toc.dy = gGlobalPrefs->tocDy;
if (toc.dy > 0) {
toc.dy = limitValue<int>(gGlobalPrefs->tocDy, 0, rc.dy);
toc.dy = limitValue(gGlobalPrefs->tocDy, 0, rc.dy);
} else {
toc.dy = rc.dy / 2; // default value
}
Expand Down
2 changes: 1 addition & 1 deletion src/SumatraPDF.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ extern bool gShowFrameRate;
extern const char* gPluginURL;
extern Favorites gFavorites;
extern WNDPROC DefWndProcCloseButton;
extern RenderCache gRenderCache;
extern RenderCache* gRenderCache;

extern bool gSupressNextAltMenuTrigger;
extern HBITMAP gBitmapReloadingCue;
Expand Down
8 changes: 5 additions & 3 deletions src/SumatraStartup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1086,6 +1086,8 @@ int APIENTRY WinMain(HINSTANCE hInstance, HINSTANCE, LPSTR, int) {

DetectExternalViewers();

gRenderCache = new RenderCache();

LoadSettings();
UpdateGlobalPrefs(flags);
SetCurrentLang(flags.lang ? flags.lang : gGlobalPrefs->uiLanguage);
Expand All @@ -1104,8 +1106,8 @@ int APIENTRY WinMain(HINSTANCE hInstance, HINSTANCE, LPSTR, int) {

gCrashOnOpen = flags.crashOnOpen;

gRenderCache.textColor = ThemeDocumentColors(gRenderCache.backgroundColor);
// logfa("retrieved doc colors in WinMain: 0x%x 0x%x\n", gRenderCache.textColor, gRenderCache.backgroundColor);
gRenderCache->textColor = ThemeDocumentColors(gRenderCache->backgroundColor);
// logfa("retrieved doc colors in WinMain: 0x%x 0x%x\n", gRenderCache->textColor, gRenderCache->backgroundColor);

gIsStartup = true;
if (!RegisterWinClass()) {
Expand Down Expand Up @@ -1399,7 +1401,7 @@ int APIENTRY WinMain(HINSTANCE hInstance, HINSTANCE, LPSTR, int) {
FreeAcceleratorTables();

FileWatcherWaitForShutdown();

delete gRenderCache;
SaveCallstackLogs();
dbghelp::FreeCallstackLogs();

Expand Down
48 changes: 48 additions & 0 deletions src/utils/BaseUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ bool AtomicRefCount::Dec() {
return res == 0;
}

void BreakIfUnderDebugger() {
if (IsDebuggerPresent()) {
DebugBreak();
}
}

void* Allocator::Alloc(Allocator* a, size_t size) {
if (!a) {
return malloc(size);
Expand Down Expand Up @@ -470,3 +476,45 @@ u32 MurmurHashStrI(const char* s) {
}
return MurmurHash2(dst - len, len);
}

int limitValue(int val, int min, int max) {
if (min > max) {
std::swap(min, max);
}
ReportIf(min > max);
if (val < min) {
return min;
}
if (val > max) {
return max;
}
return val;
}

DWORD limitValue(DWORD val, DWORD min, DWORD max) {
if (min > max) {
std::swap(min, max);
}
ReportIf(min > max);
if (val < min) {
return min;
}
if (val > max) {
return max;
}
return val;
}

float limitValue(float val, float min, float max) {
if (min > max) {
std::swap(min, max);
}
ReportIf(min > max);
if (val < min) {
return min;
}
if (val > max) {
return max;
}
return val;
}
26 changes: 9 additions & 17 deletions src/utils/BaseUtil.h
Original file line number Diff line number Diff line change
Expand Up @@ -227,16 +227,16 @@ inline void CrashMe() {
// To crash uncoditionally use ReportIf(). It should only be used in
// rare cases where we really want to know a given condition happens. Before
// each release we should audit the uses of ReportAlwaysIf()
//
// The condition is not guaranteed to be evaluated

// exclude ReportIfCond() from release builds
// in release builds ReportIf()/ReportIfQuick() will break if running under
// the debugger. In other builds it send a debug report
#undef UPLOAD_REPORT
#if defined(PRE_RELEASE_VER) || defined(DEBUG) || defined(ASAN_BUILD)
#define UPLOAD_REPORT
#endif

extern void _uploadDebugReport(const char*, bool, bool);
void BreakIfUnderDebugger();

#ifdef UPLOAD_REPORT
#define ReportIfCond(cond, condStr, isCrash, captureCallstack) \
Expand All @@ -251,6 +251,9 @@ extern void _uploadDebugReport(const char*, bool, bool);
#define ReportIfCond(cond, x, y, z) \
__analysis_assume(!(cond)); \
do { \
if (cond) { \
BreakIfUnderDebugger(); \
} \
} while (0)
#endif

Expand Down Expand Up @@ -280,20 +283,9 @@ inline void ZeroArray(T& a) {
ZeroMemory((void*)&a, size);
}

template <typename T>
inline T limitValue(T val, T min, T max) {
if (min > max) {
std::swap(min, max);
}
ReportIf(min > max);
if (val < min) {
return min;
}
if (val > max) {
return max;
}
return val;
}
int limitValue(int val, int min, int max);
DWORD limitValue(DWORD val, DWORD min, DWORD max);
float limitValue(float val, float min, float max);

// return true if adding n to val overflows. Only valid for n > 0
template <typename T>
Expand Down
2 changes: 1 addition & 1 deletion src/utils/WinUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2607,7 +2607,7 @@ bool TrackMouseLeave(HWND hwnd) {
return true;
}

// cf. http://blogs.msdn.com/b/oldnewthing/archive/2004/10/25/247180.aspx
// http://blogs.msdn.com/b/oldnewthing/archive/2004/10/25/247180.aspx
EXTERN_C IMAGE_DOS_HEADER __ImageBase;

// A convenient way to grab the same value as HINSTANCE passed to WinMain
Expand Down

0 comments on commit 54cfbb5

Please sign in to comment.