Opened 14 years ago
Closed 2 days ago
#9203 closed patch (outdated)
SDL: Avoiding pathological screen updates
| Reported by: | eriktorbjorn | Owned by: | sev- |
|---|---|---|---|
| Priority: | normal | Component: | --Other-- |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
Current ScummVM SVN This is more of a request for comments than a final patch
Currently, the SDL backend isn't particularly smart about screen updates. It will redraw all dirty rectangles (unless there are too many of them, in which case it forces a fullscreen redraw). Ideally, it might be nice if it could eliminate overlapping updates, but that sort of things is a bit outside my domain. This patch is much simpler than that: If the total area of the dirty rectangles exceeds the total area of the screen, it forces a fullscreen update.
This was prompted by the SCI engine which currently chokes badly on my computer when scrolling, unless I use the 1x scaler. I've talked to m_kiewitz about it and he said he'll try to fix it, but the patch might still be a good safety valve. Though perhaps we should wait until the OpenGL backend hits the trunk before making changes to that part of the code?
(The SCI scrolling transition currently spams the backend with lots and lots of almost fullscreen dirty rectangles, to the point where a single screen update could take 0.25 seconds for me before it finally reached the "too many; let's do a fullscreen update" theshold. Once it starts missing frames, it deteriorates quickly. I got perhaps a dozen scrolling frames, out of the desired 180, for a sideways scroll.)
Ticket imported from: #3054614. Ticket imported from: patches/1308.
Attachments (3)
Change History (12)
by , 14 years ago
| Attachment: | dirty-rect-safety-valve.diff added |
|---|
comment:1 by , 14 years ago
m_kiewitz just demonstrated a rewritten version of the SCI scrolling transition, and it works smoothly without this patch.
comment:2 by , 14 years ago
The issue this patch address still is something we need to consider.
I think we should wait till after the GSoC work has been merged (which I hope we can do right after 1.2 is out of the door.). You simple heuristic using the screen area sounds good to me; I think we might want to in addition limit the number of dirty rects. Or, we could just turn off the dirty rect handling completely. I am not quite sure how useful it really is, we would have to measure it on low-end devices run via the SDL backend. For the OpenGL SDL code path, it might also be the fastest approach. Not sure.
comment:3 by , 14 years ago
> Or, we could just turn off the dirty rect handling completely. I am not quite sure how useful it really is
It probably depends on the engine and/or situation. As an experiment, I changed the "if (_forceFull) {" condition in internUpdateScreen() to "if (_forceFull || (_numDirtyRects > 1 && getenv("SCUMMVM_FULL_REDRAW"))) {"
I started Broken Sword 2 and let the intro finish. I'm not sure how much of the screen it considers to be dirty each frame, but for me the CPU usage is 10%. Turning off dirty rects handling increased that to 20%.
This was without any scaler. I changed it to the hq2x scaler (which, in my opinion, is pretty useless for 640x480 games, but I wouldn't be surprised if some people use it). With dirty rects handling, CPU usage was 20%. Without, it was 70%; enough to make the cooling fan speed up audibly.
So I think that in some cases, dirty rects handling is still a Good Thing.
comment:4 by , 14 years ago
Come to think of it, that should probably have been "_numDirtyRects > 0". But the way Broken Sword 2 handles dirty rects, it would almost certainly be more than one anyway.
comment:5 by , 14 years ago
Just an idea: instead of doing a full screen redraw if there are too many dirty rects, why not do a single redraw of the bounding box of all the dirty rects (i.e. replace all the dirty rects by a single dirty rect that englobes all of them)?
This might update areas that are outside any dirty rects but should still in many cases be better than a full screen update I suppose
As fas as I know that's how Qt (the Nokia one, not the Apple one) handles dirty rects for widgets updates as soon as there is more than one. So I suppose this policy has some merits.
by , 14 years ago
| Attachment: | dirty-rect-safety-valve2.diff added |
|---|
Patch against current SVN (after graphics restructuring)
comment:6 by , 14 years ago
I've updated the simple patch to the current trunk. I haven't given the suggestion about using a bounding box any further thought. It might be a good idea, I guess, but the cases I've seen where this patch makes a difference, it's been trying to update most of the screen anyway.
Actually, since the SCI bug was fixed I haven't seen any case where this patch makes much of a difference (not that I've really looked), though it will trigger the area case once at the very beginning of Sam & Max, and in Curse of Monkey Island if there is a lot of animation in the background and you open the inventory.
by , 13 years ago
| Attachment: | dirty-rect-safety-valve3.diff added |
|---|
Patch against current Git (after another restructuring/renaming)
comment:7 by , 13 years ago
I've updated the patch again to current ScummVM Git. Apparently there was another renaming/restructuring of the backends.
comment:8 by , 6 years ago
| Component: | → --Other-- |
|---|
comment:9 by , 2 days ago
| Owner: | set to |
|---|---|
| Resolution: | → outdated |
| Status: | new → closed |
I am closing this as obsolete
Patch against current SVN