Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

memcpy glitches out in the web build #1875

Closed
verysoftwares opened this issue Apr 9, 2022 · 5 comments · Fixed by #2417
Closed

memcpy glitches out in the web build #1875

verysoftwares opened this issue Apr 9, 2022 · 5 comments · Fixed by #2417

Comments

@verysoftwares
Copy link

verysoftwares commented Apr 9, 2022

i'm using memcpy to move screen memory around. see the glitches here, choose 2-4 players and move around: http://tic80.com/play?cart=2740

image

a downloaded cartridge works just fine.

@nesbox
Copy link
Owner

nesbox commented Apr 10, 2022

Just reproduced it on the latest html build and I'm wondering what could be causing it, maybe compiler optimizations?

@nesbox
Copy link
Owner

nesbox commented Apr 10, 2022

No, I see the same thing in TIC-80 for desktop, are you sure your code is working correctly?

@verysoftwares
Copy link
Author

i can't reproduce on desktop (0.90.1723 Pro)

@nesbox
Copy link
Owner

nesbox commented Apr 11, 2022

I see glitches on the latest dev build
image

@mbikovitsky
Copy link
Contributor

AFAICT, there are two related issues here:

  1. The LUALA code uses memcpy to copy potentially overlapping buffers.
  2. The TIC-80 memcpy implementation uses the C memcpy function to do the copy.

It is undefined behaviour to use memcpy on overlapping buffers: https://en.cppreference.com/w/c/string/byte/memcpy. But, the documentation for the TIC-80 memcpy doesn't specify this as a constraint, so users have no way to know of this pitfall.

The immediate workaround for @verysoftwares is to copy memory manually. Something like:

function memmove(dst, src, length)
    if dst <= src then
        for i=0,length-1 do
            poke(dst+i, peek(src+i))
        end
    else
        for i=length-1,0,-1 do
            poke(dst+i, peek(src+i))
        end
    end
end

It's slower, but in my testing the bug is no longer present (AFAICT).

@nesbox, I think the correct fix is to use memmove in tic_api_memcpy, since that's the behaviour of the TIC-80 memcpy users expect.

@nesbox nesbox linked a pull request Dec 26, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants