Skip to content

Commit 5645875

Browse files
kjkkjk
committed
fix order of rendering with predictive rendering (fixes #4584)
I mistakenly commented out a code that ensured that when we queue pages for rendering, the visible page would be rendered before before / after page from preditictive rendering. Tweaked RenderCache logging. Made CONSERVE_MEMORY a runtime time switch instead of only compile time. Renamed DisplayModel::dontRenderFlag => pauseRendering.
1 parent 5c42f50 commit 5645875

5 files changed

Lines changed: 78 additions & 55 deletions

File tree

‎src/DisplayModel.cpp‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ DisplayModel::DisplayModel(EngineBase* engine, DocControllerCallback* cb) : DocC
312312
}
313313

314314
DisplayModel::~DisplayModel() {
315-
dontRenderFlag = true;
315+
pauseRendering = true;
316316
cb->CleanUp(this);
317317

318318
delete pdfSync;
@@ -1140,14 +1140,13 @@ void DisplayModel::RenderVisibleParts() {
11401140
}
11411141
}
11421142

1143-
// TODO: why the hell this is requesting pages again?
1144-
#if 0
1145-
// request the visible pages last so that the above requested
1146-
// invisible pages are not rendered if the queue fills up
1143+
// re-request the visible pages again so:
1144+
// * they get picked first by rendering thread
1145+
// * if queue fills up, the invisible pages from predictive rendering
1146+
// wont be rendered
11471147
for (int pageNo = lastVisiblePage; pageNo >= firstVisiblePage; pageNo--) {
11481148
cb->RequestRendering(pageNo);
11491149
}
1150-
#endif
11511150
}
11521151

11531152
void DisplayModel::SetViewPortSize(Size newViewPortSize) {

‎src/DisplayModel.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ struct DisplayModel : DocController {
260260
bool inPresentation = false;
261261

262262
/* allow resizing a window without triggering a new rendering (needed for window destruction) */
263-
bool dontRenderFlag = false;
263+
bool pauseRendering = false;
264264
};
265265

266266
extern bool gPredictiveRender;

‎src/RenderCache.cpp‎

Lines changed: 70 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@
3030

3131
bool gShowTileLayout = false;
3232

33+
#if defined(CONSERVE_MEMORY)
34+
bool gConserveMemory = true;
35+
#else
36+
bool gConserveMemory = false;
37+
#endif
38+
39+
static DWORD WINAPI RenderCacheThread(LPVOID data);
40+
3341
RenderCache::RenderCache() : maxTileSize({GetSystemMetrics(SM_CXSCREEN), GetSystemMetrics(SM_CYSCREEN)}) {
3442
// enable when debugging RenderCache logic
3543
// gEnableDbgLog = true;
@@ -91,11 +99,13 @@ bool RenderCache::Exists(DisplayModel* dm, int pageNo, int rotation, float zoom,
9199
}
92100

93101
bool RenderCache::DropCacheEntry(BitmapCacheEntry* entry) {
94-
ScopedCritSec scope(&cacheAccess);
95102
ReportIf(!entry);
96103
if (!entry) {
97104
return false;
98105
}
106+
107+
// TODO: make the scope of this lock smaller?
108+
ScopedCritSec scope(&cacheAccess);
99109
int idx = entry->cacheIdx;
100110
ReportIf(idx < 0);
101111
ReportIf(idx >= cacheCount);
@@ -109,8 +119,8 @@ bool RenderCache::DropCacheEntry(BitmapCacheEntry* entry) {
109119
}
110120
ReportIf(entry->refs != 0);
111121
ReportIf(cache[idx] != entry);
112-
logf("RenderCache::DropCacheEntry: pageNo: %d, rotation: %d, zoom: %.2f\n", entry->pageNo, entry->rotation,
113-
entry->zoom);
122+
logf("RenderCache::DropCacheEntry: dm: 0x%p pageNo: %d, rotation: %d, zoom: %.2f\n", entry->dm, entry->pageNo,
123+
entry->rotation, entry->zoom);
114124

115125
delete entry;
116126

@@ -242,45 +252,58 @@ static bool IsTileVisible(DisplayModel* dm, int pageNo, TilePosition tile, float
242252
of the given DisplayModel, or even all invisible pages). */
243253
void RenderCache::FreePage(DisplayModel* dm, int pageNo, TilePosition* tile) {
244254
logf("RenderCache::FreePage: dm: 0x%p, pageNo: %d\n", dm, pageNo);
255+
ReportIf(!dm || pageNo == kInvalidPageNo);
256+
if (!dm || pageNo == kInvalidPageNo) {
257+
return;
258+
}
245259
ScopedCritSec scope(&cacheAccess);
246260

247261
// must go from end becaues freeing changes the cache
248262
for (int i = cacheCount - 1; i >= 0; i--) {
249263
BitmapCacheEntry* entry = cache[i];
250-
bool shouldFree;
251-
if (dm && pageNo != kInvalidPageNo) {
252-
// a specific page
253-
shouldFree = (entry->dm == dm) && (entry->pageNo == pageNo);
254-
if (tile) {
255-
// a given tile of the page or all tiles not rendered at a given resolution
256-
// (and at resolution 0 for quick zoom previews)
257-
shouldFree =
258-
shouldFree && (entry->tile == *tile ||
259-
tile->row == (USHORT)-1 && entry->tile.res > 0 && entry->tile.res != tile->res ||
260-
tile->row == (USHORT)-1 && entry->tile.res == 0 && entry->outOfDate);
261-
}
262-
} else if (dm) {
263-
// all pages of this DisplayModel
264-
shouldFree = (entry->dm == dm);
265-
} else {
266-
// all invisible pages resp. page tiles
267-
shouldFree = !entry->dm->PageVisibleNearby(entry->pageNo);
268-
if (!shouldFree && entry->tile.res > 1) {
269-
shouldFree = !IsTileVisible(entry->dm, entry->pageNo, entry->tile, 2.0);
270-
}
264+
bool shouldFree = (entry->dm == dm) && (entry->pageNo == pageNo);
265+
if (!shouldFree && tile) {
266+
// a given tile of the page or all tiles not rendered at a given resolution
267+
// (and at resolution 0 for quick zoom previews)
268+
shouldFree = (entry->tile == *tile ||
269+
tile->row == (USHORT)-1 && entry->tile.res > 0 && entry->tile.res != tile->res ||
270+
tile->row == (USHORT)-1 && entry->tile.res == 0 && entry->outOfDate);
271271
}
272272
if (shouldFree) {
273273
DropCacheEntry(entry);
274274
}
275275
}
276276
}
277277

278+
// free all cached pages of this DisplayModel
278279
void RenderCache::FreeForDisplayModel(DisplayModel* dm) {
279-
FreePage(dm);
280+
logf("RenderCache::FreeForDisplayModel: dm=0x%p\n", dm);
281+
ScopedCritSec scope(&cacheAccess);
282+
// must go from end becaues freeing changes the cache
283+
for (int i = cacheCount - 1; i >= 0; i--) {
284+
BitmapCacheEntry* entry = cache[i];
285+
bool shouldFree = (entry->dm == dm);
286+
if (shouldFree) {
287+
DropCacheEntry(entry);
288+
}
289+
}
280290
}
281291

282292
void RenderCache::FreeNotVisible() {
283-
FreePage();
293+
// logf("RenderCache::FreeNotVisible\n");
294+
ScopedCritSec scope(&cacheAccess);
295+
// must go from end becaues freeing changes the cache
296+
for (int i = cacheCount - 1; i >= 0; i--) {
297+
BitmapCacheEntry* entry = cache[i];
298+
// all invisible pages resp. page tiles
299+
bool shouldFree = !entry->dm->PageVisibleNearby(entry->pageNo);
300+
if (!shouldFree && entry->tile.res > 1) {
301+
shouldFree = !IsTileVisible(entry->dm, entry->pageNo, entry->tile, 2.0);
302+
}
303+
if (shouldFree) {
304+
DropCacheEntry(entry);
305+
}
306+
}
284307
}
285308

286309
// keep the cached bitmaps for visible pages to avoid flickering during a reload.
@@ -415,12 +438,12 @@ void RenderCache::RequestRendering(DisplayModel* dm, int pageNo) {
415438

416439
/* Render a bitmap for page <pageNo> in <dm>. */
417440
void RenderCache::RequestRendering(DisplayModel* dm, int pageNo, TilePosition tile, bool clearQueueForPage) {
418-
logf("RenderCache::RequestRendering(): pageNo %d\n", pageNo);
419-
ScopedCritSec scope(&requestAccess);
441+
logf("RenderCache::RequestRendering: pageNo %d\n", pageNo);
420442
ReportIf(!dm);
421-
if (!dm || dm->dontRenderFlag) {
443+
if (!dm || dm->pauseRendering) {
422444
return;
423445
}
446+
ScopedCritSec scope(&requestAccess);
424447

425448
int rotation = NormalizeRotation(dm->GetRotation());
426449
float zoom = dm->GetZoomReal(pageNo);
@@ -480,9 +503,10 @@ void RenderCache::Render(DisplayModel* dm, int pageNo, int rotation, float zoom,
480503

481504
bool RenderCache::Render(DisplayModel* dm, int pageNo, int rotation, float zoom, TilePosition* tile, RectF* pageRect,
482505
const OnBitmapRendered* renderCb) {
483-
logf("RenderCache::Render(): pageNo %d\n", pageNo);
506+
logf("RenderCache::Render: pageNo %d\n", pageNo);
484507
ReportIf(!dm);
485-
if (!dm || dm->dontRenderFlag) {
508+
if (!dm || dm->pauseRendering) {
509+
logf(" skipped because dm->pauseRendering\n");
486510
return false;
487511
}
488512

@@ -633,7 +657,7 @@ void RenderCache::AbortCurrentRequest() {
633657
curReq->abort = true;
634658
}
635659

636-
DWORD WINAPI RenderCache::RenderCacheThread(LPVOID data) {
660+
static DWORD WINAPI RenderCacheThread(LPVOID data) {
637661
RenderCache* cache = (RenderCache*)data;
638662
PageRenderRequest req;
639663
RenderedBitmap* bmp;
@@ -655,7 +679,7 @@ DWORD WINAPI RenderCache::RenderCacheThread(LPVOID data) {
655679
continue;
656680
}
657681

658-
if (req.dm->dontRenderFlag) {
682+
if (req.dm->pauseRendering) {
659683
if (req.renderCb) {
660684
req.renderCb->Call(nullptr);
661685
}
@@ -674,18 +698,20 @@ DWORD WINAPI RenderCache::RenderCacheThread(LPVOID data) {
674698
RenderPageArgs args(req.pageNo, req.zoom, req.rotation, &req.pageRect, RenderTarget::View, &req.abortCookie);
675699
auto timeStart = TimeGet();
676700
bmp = engine->RenderPage(args);
701+
auto durMs = TimeSinceInMs(timeStart);
677702
if (req.abort) {
703+
logf("RenderCacheThread: aborted rendering page %d in %.2f ms\n", req.pageNo, (float)durMs);
678704
delete bmp;
679705
if (req.renderCb) {
680706
req.renderCb->Call(nullptr);
681707
}
682708
continue;
683709
}
684-
auto durMs = TimeSinceInMs(timeStart);
685710
if (durMs > 100) {
686711
auto path = engine->FilePath();
687712
logfa("Slow rendering: %.2f ms, page: %d in '%s'\n", (float)durMs, req.pageNo, path);
688713
}
714+
logf("RenderCacheThread: rendered page %d in %.2f ms\n", req.pageNo, (float)durMs);
689715

690716
if (req.renderCb) {
691717
// the callback must free the RenderedBitmap
@@ -869,18 +895,18 @@ int RenderCache::Paint(HDC hdc, Rect bounds, DisplayModel* dm, int pageNo, PageI
869895
}
870896
}
871897

872-
#ifdef CONSERVE_MEMORY
873-
if (!neededScaling) {
874-
if (renderOutOfDateCue) {
875-
*renderOutOfDateCue = false;
898+
if (gConserveMemory) {
899+
if (!neededScaling) {
900+
if (renderOutOfDateCue) {
901+
*renderOutOfDateCue = false;
902+
}
903+
// free tiles with different resolution
904+
TilePosition tile(targetRes, (USHORT)-1, 0);
905+
// logf("RenderCache::Paint: calling FreePage() pageNo: %d\n", pageNo);
906+
FreePage(dm, pageNo, &tile);
876907
}
877-
// free tiles with different resolution
878-
TilePosition tile(targetRes, (USHORT)-1, 0);
879-
logf("RenderCache::Paint: calling FreePage() pageNo: %d\n", pageNo);
880-
FreePage(dm, pageNo, &tile);
908+
FreeNotVisible();
881909
}
882-
FreeNotVisible();
883-
#endif
884910

885911
return renderDelayMin;
886912
}

‎src/RenderCache.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,10 @@ struct RenderCache {
142142
void ClearQueueForDisplayModel(DisplayModel* dm, int pageNo = kInvalidPageNo, TilePosition* tile = nullptr);
143143
void AbortCurrentRequest();
144144

145-
static DWORD WINAPI RenderCacheThread(LPVOID data);
146-
147145
BitmapCacheEntry* Find(DisplayModel* dm, int pageNo, int rotation, float zoom = kInvalidZoom,
148146
TilePosition* tile = nullptr);
149147
bool DropCacheEntry(BitmapCacheEntry* entry);
150-
void FreePage(DisplayModel* dm = nullptr, int pageNo = -1, TilePosition* tile = nullptr);
148+
void FreePage(DisplayModel* dm, int pageNo, TilePosition* tile);
151149
void FreeNotVisible();
152150

153151
int PaintTile(HDC hdc, Rect bounds, DisplayModel* dm, int pageNo, TilePosition tile, Rect tileOnScreen,

‎src/SumatraPDF.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2730,7 +2730,7 @@ void CloseWindow(MainWindow* win, bool quitIfLast, bool forceClose) {
27302730

27312731
for (auto& tab : win->Tabs()) {
27322732
if (tab->AsFixed()) {
2733-
tab->AsFixed()->dontRenderFlag = true;
2733+
tab->AsFixed()->pauseRendering = true;
27342734
}
27352735
}
27362736

0 commit comments

Comments
 (0)