Fixes ppu_modes_timing.svg to more properly reflect the description b… #355
Conversation
…ox above. Also added extra detail in the description box found under Rendering -> Pixel FIFO to more accurately explain what is meant by a 'dot'.
src/pixel_fifo.md
Outdated
| certain action *lengthens mode 3* it means that mode 0 (hblank) is | ||
| All references to a cycle or dot are meant as T-cycles (4.19 MHz) and cycle | ||
| counts are doubled on CGB in double speed mode. Dots are the terminology used | ||
| for cycles in the context of rendering, and are synonymous with a T-cycle. |
There was a problem hiding this comment.
Dots are not synonymous, as they stay at 4 MiHz even when double-speed causes T-cycles to be at 8 MiHz. Hence why a different unit is used.
This terminology issue generally overlaps with #194.
There was a problem hiding this comment.
Really, then, the word "cycle" should be removed from the page and replaced with "dot", since it is really the unit being used throughout the document (just with a wrong name). Then this box can be simplified to just explaining what a "dot" is.
There was a problem hiding this comment.
Overlooking the typo 'perscanline'
'All references to a cycle are meant as T-cycles (4.19 MHz) and cycle counts are doubled on CGB in double speed mode. On DMG, a dot is equal to a single T-cycle. However, for CGB in double speed a dot is 2 T-cycles as to keep the amount of dots perscanline and frame similar regardless of processor speeds. When it is stated that a certain action lengthens mode 3 it means that mode 0 (hblank) is shortened to make up for the additional time in mode 3, as shown in the following diagram.'
Should the term cycle be omitted from here? Does it cause confusion over the meaning of a 'dot'?
There was a problem hiding this comment.
T-cycles do not intervene in any of the document. T-cycles and M-cycles are bound to the CPU clock, which speeds up when double-speed mode is enabled.
Dots, however, stem from another clock which is not affected by double-speed mode. This is why they are a separate unit: because they are not relative to the CPU speed.
There was a problem hiding this comment.
The problem comes then later down the line when T-cycles are used the measure the duration of, for instance, the the pixel fetcher's different steps.
'The fetcher fetches a row of 8 background or window pixels and queues them up to be mixed with sprite pixels. The pixel fetcher has 5 steps. The first four steps take 2 cycles each and the fifth step is attempted every cycle until it succeeds.'
I could gladly omit the reference to T-cycles and just instead give an overview of "dot", but I personally think it's beneficial to give the insight that, on a normal DMG, a single dot seems to equate to a sinlge T-cycle. Although again I could be very wrong on this and I don't have the specific clock rate for the PPU. I could completely omit the T-cycle parallel and just start explaining, and researching, the frequency of the PPU clock and only explain dots in via the PPU clock rather than any parallels to the CPU, if that's preferred? Either way, originally the diagram used to have M-cycles to explain the timings of the different modes. Or at least it seemed to be M-cycles or what was otherwise meant by the "(= cycles)" comments? I didn't want to omit any original information just clarify what was there.
There was a problem hiding this comment.
As I explained above, the unit used throughout the document is really dots, not T-cycles.
There was a problem hiding this comment.
Ah my bad. I see now what you mean.
There was a problem hiding this comment.
Great! That's much better, but now there's a needlessly confusing alias. Could you try replacing all occurrences of "cycle" with "dot" in the document?
There was a problem hiding this comment.
For sure! I'm on it!
There was a problem hiding this comment.
should be fixed now! :)
…nation of the meaning behind a dot.
There was a problem hiding this comment.
A part from the mentioned suggestion, LGTM! Thanks @Kim-Dewelski
…rather than T-cycles. Fixed spelling of "hblank" to "HBlank".
|
Small word of update: I realized that some of the document's wording needed changing, and the scope crept into a rewrite of the entire article. Hope you won't mind—the changes already brought in are good, it's just that it's a good time as ever to do this. |
Isn't that feasible in the second moment? Can we start merging this then iterate over later? |
|
My usual concerns apply, but if it's like last time, then sure, okay to merge. |
Yep, I do think it's reasonable to merge and iterate afterwards. Can you check if the last thread here looks resolved to you? |
|
The fact that it isn't is how I came to rewrite the entire article :P If we're planning to merge the changes I'm working on quickly, there's no reason to dwell on this, anyway. |
…ox above. Also added extra detail in the description box found under Rendering -> Pixel FIFO to more accurately explain what is meant by a 'dot'.