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

MEGA65: Characters not drawn when started at negative GOTOX values #356

Open
smnjameson opened this issue Sep 5, 2022 · 9 comments
Open

Comments

@smnjameson
Copy link
Contributor

smnjameson commented Sep 5, 2022

When the VIC4 attempts to render a sequence of characters that sstarts offscreen to the left (eg gotox = $03f0), it fails to do so. It seems like if after a single char the x position is out of range it terminates rendering until another GOTOX or end of line

Xemu version : 20220831211537

Please see attached videos and test.prg
test.zip

Devkit Expected:

hardware.mp4

Xemu Actual:

xemu.mp4
@lgblgblgb
Copy link
Owner

lgblgblgb commented Sep 26, 2023

Errr, this seems to be a long-stalled issue here, trying to catch-up a "bit" ... Reading the issue again, at first glance it seems to me that Xemu handles position as an unsigned entity, $3f0 for Xemu is at right not at left. In any way, I think the problem indeed, that the value does not "overflow" to zero when going above $3ff. Or something like that. I'll have a look.

EDIT:

Maybe there will be a fundamental issue here which makes this really hard to emulate (at least within the current structure of the emulator), since xcounter in Xemu used for many structures and never can be negative ... And somehow it should be handled as negative still. But in that case almost all rendering codepath must be "decorated" by checks, significantly slowing down performance. The same problem with any render-overflow/render-underflow topic, like sprite at extreme right (and X-expanded) seen on the left: this is also impossible to do in Xemu currently at least not in a sane way ...

Also, Xemu's approach of "direct rendering" into SDL texture makes it hard, as negative values would mean literally to overwrite the previous also rendered scanline. Unless, if "expensive" checks is made every single places into the rendering pipeline.

@lgblgblgb lgblgblgb self-assigned this Sep 26, 2023
@lgblgblgb
Copy link
Owner

I've tried to solve this issue several times (I always think, a "fresh start" / "fresh ideas" can help, from time to time) already but always failed. I can make the test program working as expected, but it has serious ill effects on every other things, even on the default READY. screen on BASIC65. This is very hard to handle, since in Xemu, there is no conception on negative values in this context, so the "negative" is seen as a large positive value instead and needs several hacks. Where one/some may fail to do what I thought. Probably this requires a bit deeper dive and modify how the position info is handled, everywhere in VIC-IV emulation ...

@lgblgblgb
Copy link
Owner

lgblgblgb commented Jan 16, 2025

Note to myself: new idea. Currently vic4.c uses two separated variables in rendering: current_pixel is an Uint32* pointer to the texture which is also used as the RRB buffer directly and points to the actually rendered scanline. Also xcounter is what the name suggests. These two set/incremented together all the time. Probably, I should git rid off current_pixel and use pixel_raster_start[xcounter] instead, with the knowledge though, that xcounter should be clamped to 10 bit (& 1023 ?) either at setting/incrementing (xcounter = (xcounter + 1) & 1023;), or at reference (pixel_raster_start[xcounter & 1023] instead of pixel_raster_start[xcounter]).

pixel_raster_start[xcounter] and *current_pixel in theory should mean the same if it's true that xcounter and current_pixel is always managed together. Maybe, as a first step, I should make some asserts placed "everywhere" to check this not to miss some hidden place/case when this may not be the case. So in theory the condition pixel_raster_start + xcounter == current_pixel should hold everywhere for my theory being applicable then.

Also a bonus, that I don't need to maintain two variables which are about the same meaning and always managed together.

This may also cure the problem mentioned here (mentioned by @bjotos (btoschi on MEGA65 Discord):

https://discord.com/channels/719326990221574164/977076334297743360/1325574298903117824

Another thread mentioned in the above Discord message:

https://discord.com/channels/719326990221574164/977076334297743360/1325569188265332777

However I should test, that my assumptions are right, and this can work as I imagine without breaking something in the emulator or compatibility with some MEGA65 software.

This issue is extremely useful as it includes a dedicated test program written by @smnjameson (thanks) so I can test if a proposed fix really works.

Also some TODOs:

  • is_fg[] and is_sprite[] can be done then to 1024 bytes long without any comment about worrying overflow
  • clean-up code / comment in the GOTOX section about the mentioned worrying and implementation in general
  • clean-up comments at the declaration of various pixel pointers

@lgblgblgb
Copy link
Owner

lgblgblgb commented Jan 16, 2025

I've just tried this approach locally (not committed). It's a quite big change, at the first glance the emulator seems to work normally still. However the problem hasn't been fixed at all. So I guess, it's not (or not only?) about the clamped RRB position to 10 bit problem, but some condition is wrong in the VIC4 emulation ... Probably some "if" condition when too big value if handled as "over" some limit already and not interpreted as "negative" (ie. thinking it's already done, not rendering anything, since xcounter is already over the limit).

So, it seems, this issue is still a tough one, it can't give itself up easily, even not after multiple different tries of mine to be solved.

Anyway, I need further investigate for the mentioned rewrite if it has performance and/or other benefits (and if it fixes anything, even if not this issue) to worth to keep it regardless.

@lgblgblgb
Copy link
Owner

lgblgblgb commented Jan 17, 2025

OK. It seems the problem exists within the actual renderer functions which - for example in case of the mono char renderer - has a loop like this:

for (float cx = 0; cx < glyph_width && xcounter < border_x_right; cx += char_x_step) {
    [...]
    xcounter = (xcounter + 1) & 1023;   // INC_PIXEL(); in the new code (resolving to this clamped stuff), xcounter++; in the older one
}

The problem here: if xcounter is outside of the border already, this loop never runs (xcounter < border_x_right is false already). And the loop itself increments xcounter, so it never overflows to zero (after 1023) this way but stays as is.

With the following ad-hoc modification only for the mono char renderer for now (what the test program uses):

for (float cx = 0; cx < glyph_width; cx += char_x_step) 

We can see this:

Image

It's much better now, however that last column filed with @ characters shouldn't happen (at least it's not on the original video posted by @smnjameson [Shallan]). Also the MEGA65 version of the video contains a vertical line at the left, which I have no idea about either. -- UPDATE: it seems I broke the border painting algorithm, the @ column should be covered by the border! See below on my next comment-block.

Anyway, it seems, the drop of the current_pixel and clamping the xcounter to a 10 bit value is otherwise very useful, as without that, this modification wouldn't work at all.

Note: dropping the xcounter < border_x_right condition otherwise is not a problem, as the border area will be rendered at the end, so affected texture space will be filled anyway, if it's border area (or if it's a large value near to 1023 then evem it's the next scanline already - fear not, border will be filled, and next scanline will be rendered over the proper place then - this is because in Xemu the RRB buffer is actually the target texture's current raster not a separated buffer).

@lgblgblgb
Copy link
Owner

lgblgblgb commented Jan 17, 2025

Please help

I'm looking for other software, test programs, which may not work currently in Xemu, so I can test with my tries here

Also, any discussion is very much welcome here, including ideas what is that vertical line on the real hardware, or why does @ happens in Xemu (I have some faint memories that RRB needs some "terminating" GOTOX to work properly on a real hw, or something like that, but I am very much not sure, and even if it's needed, it shouldn't be visible as @ chars??)

Of course any other idea, suggestion, comment is also welcome.

Findings

Image

@lgblgblgb
Copy link
Owner

Meanwhile: I've fixed the border painting algorithm.

@lgblgblgb
Copy link
Owner

lgblgblgb commented Jan 20, 2025

The work - so far - is in dev branch already, commit 0c0d50b

@RetroCogs test program (thanks!) seems to work nicely now:

The test program:

tutorial_3a_fcm_rrb.zip

Before my modifications:

Image

After my modifications:

Image

@lgblgblgb
Copy link
Owner

FIXME: Known issue still (and who knows there is more): Shallan's VIC4tests in menu 'B' then 'E': for a while the "bar" disappears. It didn't happened with the old (but incorrect) code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

No branches or pull requests

2 participants