Opened 10 years ago

Closed 10 years ago

Last modified 20 months ago

#4860 closed defect (fixed)

DW2: graphical errors in video on Windows

Reported by: bluddy Owned by: fingolfin
Priority: normal Component: Engine: Tinsel
Keywords: Cc:
Game: Discworld II

Description

Video in DW2 has serious artifacts on Windows as of version 1.1.0. Works fine in 1.0.0. Apparently this problem doesn't appear on Linux, and it's fine on the PSP too.

Attaching images to show issues. Note that some videos (like the logos) show fine.

Ticket imported from: #2987895. Ticket imported from: bugs/4860.

Attachments (4)

dw1.jpg (19.4 KB ) - added by bluddy 10 years ago.
dw2.jpg (28.4 KB ) - added by bluddy 10 years ago.
dw3.jpg (28.7 KB ) - added by bluddy 10 years ago.
dw4.jpg (23.7 KB ) - added by bluddy 10 years ago.

Download all attachments as: .zip

Change History (14)

by bluddy, 10 years ago

Attachment: dw1.jpg added

by bluddy, 10 years ago

Attachment: dw2.jpg added

by bluddy, 10 years ago

Attachment: dw3.jpg added

by bluddy, 10 years ago

Attachment: dw4.jpg added

comment:1 by lordhoto, 10 years ago

Just continuing the discussion from -devel:

I noticed I am also able to reproduce similar glitches with valgrind. When I use memmove instead of memcpy they are gone though. Also using the code in the other #if case (i.e. just replace all "#if 1" by "#if 0" in bmv.cpp) fixes it for me too. Maybe you can give that a try too?

comment:2 by lordhoto, 10 years ago

Gave it another run here are my results for the death preparation intro:

1) memcpy: Works fine without valgrind, bugged with valgrind. 2) memmove: Is bugged with and without valgrind (guess I somehow forgot to rebuild in my former test runs :-/, sorry about that) 3) the #else code path works fine too

I guess on both valgrind and Windows memcpy is implemented similar to memmove, i.e. it preserves overlapping memory data, while the second code path and glibc's normal memcpy implementation probably just copy from start to end thus overwrite the overlapping data.

Please give the #else code path a try if that works, I guess we should switch to that (even in case it might be less customizable/optimized by the compiler).

comment:3 by lordhoto, 10 years ago

Owner: set to fingolfin

comment:4 by lordhoto, 10 years ago

Assigning to Max since according to eriktorbjorn r45397 broke it.

comment:5 by fingolfin, 10 years ago

Just remove the #if 1 parts and use the #else code instead (and backport it to the 1.1.x branch). I can look into doing that tomorrow, but for now I'll got get some sleep.

comment:6 by fingolfin, 10 years ago

This should be fixed on trunk and branch now, please verify.

comment:7 by fingolfin, 10 years ago

Resolution: fixed
Status: newclosed

comment:8 by bluddy, 10 years ago

Yeah works fine now.

So we have overlapping memcopy's where the source is SUPPOSED to be overwritten by the destination, right? And memmove prevents this, as does the Windows memcpy? So maybe we should implement our own memcpy (as I do in the PSP backend) ? The savings in processing time should be worth it.

comment:9 by lordhoto, 10 years ago

I am not sure why we would want our own memcpy which behaves like that though? I doubt there's many use cases for that and in general the system specific memcpy might be faster due to certain optimizations. Also the memcpy behavior, which was required here does not match the standard C' memcpy behavior (i.e. there it is undefined what happens to memory overlapping), so that is imho no reason to write our own memcpy, which enforces a behavior different to the standard memcpy and with the same name. Of course one might consider to implement a memcpy similar function (with a different name of course!) with that specific behavior, but as I said I fail to see the uses cases, is there any other code which would benefit from that?

comment:10 by digitall, 20 months ago

Component: Engine: Tinsel
Game: Discworld II
Note: See TracTickets for help on using tickets.