lronaldo / cpctelera

Astonishingly fast Amstrad CPC game engine for C developers
http://lronaldo.github.io/cpctelera/
GNU Lesser General Public License v3.0
229 stars 54 forks source link

cpct_px2byteM0 not working properly when accessing dc_mode0_ct table produces an 8 bit overflow #85

Closed johnlobo closed 6 years ago

johnlobo commented 6 years ago

In the code of cpct_px2byteM0 there is an initialization of the "a" registry using "sub a", apparently because in that way the carry flag is preserved... but in my code "sub a" actually resets the carry flag, what is generating a random number as reesult of the cpct_px2byteM0 function when the dc_mode0_ct is not 8 bit aligned.

;; Pixel colour table defined in cpct_drawCharM0 .globl dc_mode0_ct

;; Macro that computes A = *(DE + A) ;; to access vales stored in tables pointed by DE ;; .macro A_eqDEplusA ;; Compute DE += A add e ;; [1] | E += A ld e, a ;; [1] | sub a ;; [1] A = 0 (preserving Carry Flag) <---------------- HERE adc d ;; [1] | D += Carry ld d, a ;; [1] |

;; A = *(DE + A) ld a, (de) ;; [2] A = Value stored at the table pointed by DE .endm ....

It could be fixed replacing "sub a" by "ld a, 0".

lronaldo commented 6 years ago

You are completely right, @johnlobo. This DE += Aoperation is badly implemented and "sub a" always produces Carry=0, effectively not preserving carry flag.

ld a, #0 solves the issue, but takes 1 byte + 1 microsecond more. Solution in macro add_de_a from cpct_maths.h.s is better. sub a : adc d is replaced by adc d : sub e which works as expected, taking same amount of bytes and cycles.

This solution has been pushed to current development branch. You may add the fix manually by yourself or update to development branch.

Thank you very much for reporting.

lronaldo commented 6 years ago

Follow-up: As it is a bug, I have also pushed the fix to current master branch. I think it is important enough to be also fixed there.

johnlobo commented 5 years ago

Thank you Fran.

I've tried to build the new version in master branch, but I'm getting an error when building z80 lib..

/Users/david/Documents/retro/proyectos/cpctelera/cpctelera/tools/sdcc-3.6.8-r9946/bin/sdasz80 -l -o -s obj/sprites/cpct_px2byteM0_asmbindings.rel src/sprites/cpct_px2byteM0_asmbindings.s obj/sprites/cpct_px2byteM0_asmbindings.sym:92: Error: .include file error or an .if/.endif mismatch obj/sprites/cpct_px2byteM0_asmbindings.sym:97: Error: missing or improper operators, terminators, or delimiters obj/sprites/cpct_px2byteM0_asmbindings.sym:105: Error: missing or improper operators, terminators, or delimiters removing obj/sprites/cpct_px2byteM0_asmbindings.rel make: *** [obj/sprites/cpct_px2byteM0_asmbindings.rel] Error 2

Operating system: macOS Mojave v.10.14

lronaldo commented 5 years ago

Thank you again for your revision @johnlobo. I made a mistake on the fix. The error is not due to your operative system, but due to cpct_maths.h.s not being available on master branch. It is only present on current development branch. As I cherry-picked the fixing commit from development branch, it didn't work on master branch.

I've manually added the definition of the macro in the master branch and it should work now. If you could confirm it works, it would be nice.