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

Overhaul "Pixel FIFO" article into "Rendering Internals" #379

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ISSOtm
Copy link
Member

@ISSOtm ISSOtm commented Oct 15, 2021

Also avoid describing SameBoy internals, instead relying on it when otherwise corroborated, or on schematics and/or test ROMs when possible.

Restructure the article to describe behavior more than components, especially in a way that is more friendly to someone not knowing what all the components are about.

Add a diagram, too, and move the mode timing diagram to the STAT article, where it belongs just as well, but where it will be more visible and thus more useful.

Fixes #377, fixes #408.

@ISSOtm ISSOtm added content Improvements or additions to documentation enhancement New feature or request labels Oct 15, 2021
src/Rendering_Internals.md Outdated Show resolved Hide resolved
src/Rendering_Internals.md Outdated Show resolved Hide resolved
src/Rendering_Internals.md Show resolved Hide resolved
src/Rendering_Internals.md Outdated Show resolved Hide resolved
src/Rendering_Internals.md Outdated Show resolved Hide resolved
@ISSOtm
Copy link
Member Author

ISSOtm commented Sep 24, 2022

I rebased the branch and applied some editorial changes; the article is still not finished, and should not be reviewed yet. (At least not before #350 is merged, so we can focus on that.)

Copy link
Sponsor Member

@avivace avivace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just gave it a quick look.

Looks nice so far!

src/STAT.md Outdated
@@ -2,7 +2,7 @@

::: tip TERMINOLOGY

A *dot* is the shortest period over which the PPU can output one pixel: is it equivalent to 1 T-state on DMG or on CGB single-speed mode or 2 T-states on CGB double-speed mode. On each dot during mode 3, either the PPU outputs a pixel or the fetcher is stalling the [FIFOs](<#Pixel FIFO>).
A *dot* is the shortest period over which the PPU can output one pixel: is it equivalent to 1 T-state on DMG or on CGB single-speed mode or 2 T-states on CGB double-speed mode. On each dot during mode 3, either the PPU outputs a pixel or the fetcher is stalling the [FIFOs](<#Rendering Internals>).
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
A *dot* is the shortest period over which the PPU can output one pixel: is it equivalent to 1 T-state on DMG or on CGB single-speed mode or 2 T-states on CGB double-speed mode. On each dot during mode 3, either the PPU outputs a pixel or the fetcher is stalling the [FIFOs](<#Rendering Internals>).
A *dot* is the shortest period of time over which the PPU can output one pixel: is it equivalent to 1 T-state on DMG or on CGB single-speed mode or 2 T-states on CGB double-speed mode. On each dot during mode 3, either the PPU outputs a pixel or the fetcher is stalling the [FIFOs](<#Rendering Internals>).

<text x="99" y="419">(9-bit tile ID, Y offset)</text>
<rect x="90" y="429" class="legend pxrow"/>
<text x="99" y="433">Pixel row (2 bytes)</text>
<text x="465" y="405" class="right">(Some arrows have been</text>
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would put this text way smaller (and maybe in a angle?)

- The BG FIFO is not empty

Once both conditions are fulfilled, the OBJ FIFO takes over, discarding the pixels slices already fetched.
Note that if the BG FIFO is empty, the Pixel Slice Fetcher immediately switches to [Get tile ID](<#Get tile ID>) when refilling it, so the OBJ fetcher will wait for 6 additional dots.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to my understanding of the sprite timing, the maximum "penalty" for a sprite occurring too soon in the BG slice fetch sequence is 5 dots (making that sprite contribute at most 11 dots to mode 3). I'm not clear on the technical reason behind this, though.

This understanding is corroborated by https://gbdev.io/pandocs/STAT.html#properties-of-stat-modes and https://www.reddit.com/r/EmuDev/comments/59pawp/gb_mode3_sprite_timing/

- The Pixel Slice Fetcher is attempting to [push pixels](<#Push pixels>)
- The BG FIFO is not empty

Once both conditions are fulfilled, the OBJ FIFO takes over, discarding the pixels slices already fetched.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What pixel slices are discarded? Are they refetched later, and when is there time for that? I assumed the BG fetching sequence latches the fetched slices somewhere until it's able to successfully push them to the BG FIFO (for which it always has to wait at least 2 dots anyway).

@kapoisu
Copy link

kapoisu commented Jan 13, 2023

I'm going to propose some feedback about why I've had a hard time understanding this document from the point of view of a novice.

Especially in the Sprite part, there are terms like "X coordinate" which I'm not sure what they refer to. From the aspect of implementation, I reset the counter of the fetcher when I enter the window. Hence I can't compare the X coordinate of the sprite to it and it's reasonable to think that these terms refer to a counter which tracks the number of pixels actually shifted out and pushed to the LCD.

The assumption above is valid only if I keep as less states as possible. What if I had an extra counter which tracks the number of pixels already fetched? The timing of the sprite checking would be changed from "after a pixel is popped" to "before a pixel is fetched." This may not be true (to be honest I'm not that confident) but it's actually a reasonable consequence - because I have to do the sprite checking, I introduce this extra counter.

That is to say, I suppose the term is a little bit too brief because I can consider all of the counters mentioned above sorts of "X coordinate."

In the OBJ fetcher part,

the OBJ fetcher waits to take control until two conditions are met:

  • The Pixel Slice Fetcher is attempting to push pixels
  • The BG FIFO is not empty

Firstly, what is the exact timing of "attempting to push pixels?" Is it the 6th, 7th, or 8th cycle of the fetcher? From my understanding, this would affect the number of cycles added to mode 3.

I've learned - from somewhere else - that there may be delay up to 11 cycles for each sprite. There are descriptions such as "shorten/lengthen mode 3 by n dots" spread across the document. My point is, they are not tidied up and summarized in an easy-to-understand way. How/Why these conditions contribute to the max 11-cycle delay and when these conditions are checked aren't stated clearly.

Use this page as an example. 172-289 dots? Readers are not going to figure out 289 = 12 (before shifter is filled) + 160 (pixels per scanline) + 7 (max(SCX % 8)) + 11 (sprite delay) * 10 (number of sprites) from pure imagination (and I don't even know why a possible window restart is not included.)

With all the ambiguities combined, I can't even have a strong guarantee of the correctness of the overall control flow.

Every time both FIFOs are clocked, the selector decides whether to retain the pixel from the BG or OBJ FIFO.

The selection follows the following rules:
1. **In CGB Mode**, if [`LCDC` bit 0 (priority enable)](<#CGB Mode: BG and Window master priority>) is reset, pick the BG pixel.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rule appears to be simply disabling OBJ, which duplicates the functionality of rule 3 and doesn't seem correct. I believe a correct rule would be to ignore rules 4 and 5.

Also avoid describing SameBoy internals, instead relying on it when
otherwise corroborated, or on schematics and/or test ROMs when possible.

Restructure the article to describe behavior more than components, especially
in a way that is more friendly to someone not knowing what all the components
are about.

Add a diagram, too, and move the mode timing diagram to the STAT article, where
it belongs just as well, but where it will be more visible and thus more useful.
@ISSOtm ISSOtm added the help wanted Extra attention is needed label Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Contradictions in PPU FIFO size Get Tile step during the FIFO needs correction / improvement
4 participants