Skip to content

Add ram viewer.#2003

Merged
nicolasnoble merged 9 commits intogrumpycoders:mainfrom
nicolasnoble:ram-viewer
Apr 15, 2026
Merged

Add ram viewer.#2003
nicolasnoble merged 9 commits intogrumpycoders:mainfrom
nicolasnoble:ram-viewer

Conversation

@nicolasnoble
Copy link
Copy Markdown
Member

No description provided.

GPU-accelerated visualization of the PS1's main RAM (2MB/8MB) as a
2048-wide 2D texture with live read/write/execute heatmap overlays.

The heatmap stores per-byte cycle timestamps from the CPU debug hooks,
uploaded as GL_R32UI textures. The fragment shader computes exponential
decay from the current cycle counter, so the visualization is frame-rate
independent and never loses access history until overwritten.

Features:
- Three color-coded channels: read (green), write (red), execute (blue)
- Configurable decay half-life
- Byte-level grid overlay at high zoom
- Hex byte value display using the ImGui font atlas as a glyph texture
- Auto-contrast text (white on dark, black on light)
- Pan/zoom with shared ZoomableImage base class
- Bidirectional integration via event bus: click in RAM viewer jumps
  hex editor to that address, RAMFocus event zooms RAM viewer to a
  target address range

Signed-off-by: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org>
The ImGui font atlas is RGBA32 with glyph shapes in the alpha channel
(RGB is white for tinting). The shader was sampling .r instead of .a,
producing solid white blocks instead of readable hex digits.

Signed-off-by: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org>
The glyphs were filling 80% of each cell, leaving almost no margin.
Reduced to 50% height with proportional width and centered both
horizontally and vertically within the cell.

Signed-off-by: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org>
ImGui's render loop binds pcmd->GetTexID() to GL_TEXTURE_2D before
drawing each command. Passing 0 as the texture ID caused ImGui to
overwrite the RAM texture binding with the null texture, resulting
in all-zero reads in the shader.

Pass the actual RAM texture handle so ImGui's bind is consistent
with the shader's expectations on texture unit 0.

Signed-off-by: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org>
When disabled, the base color is treated as black (as if RAM byte = 0),
leaving only the heatmap colors visible. Useful for isolating access
patterns without the underlying data values competing visually.

Signed-off-by: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org>
The RAM logger only tracked CPU load/store instructions via
debug.cc::process(). DMA transfers (GPU, SPU, CD-ROM, MDEC, etc.)
bypass the CPU instruction path and go through checkDMAread/
checkDMAwrite instead. These now feed into the RAM logger so
DMA activity is visible in the heatmap.

Signed-off-by: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org>
Completes the extraction started when ZoomableImage was created.
VRAMViewer now shares the zoom/pan state members and zoom() method
with RAMViewer via the common base class.

VRAMViewer keeps its own moveTo() (VRAM-specific UV-space transform),
resetView() override (adds magnifier reset), and the magnifier-aware
scroll handling in drawVRAM (ctrl+scroll adjusts magnifier instead
of zoom).

Signed-off-by: Nicolas 'Pixel' Noble <nicolas@nobis-crew.org>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 14, 2026

📝 Walkthrough

Walkthrough

Adds a RAM logging system that records per-byte read/write/execute timestamps, GPU-backed heatmaps, and a shader-driven RAM viewer widget; integrates logging into CPU/DMA/debug paths, wires a RAMLogger into the Emulator, and adds shared zoom/pan infrastructure for viewers.

Changes

Cohort / File(s) Summary
Core: RAM logger
src/core/ramlogger.h, src/core/ramlogger.cc
New PCSX::RAMLogger with per-byte timestamp arrays (Read/Write/Execute), OpenGL textures (RAM + 3 heatmaps), recordAccess(), enable/disable, uploadRAM()/uploadHeatmaps(), and decay config.
CPU / Debug integration
src/core/debug.h, src/core/debug.cc
Added logDMAAccess(...); calls to recordExecution/read/write accesses when normalized addresses map to RAM for instruction execute, loads/stores, and DMA.
Emulator wiring
src/core/psxemulator.h, src/core/psxemulator.cc
Forward-declared RAMLogger, added std::unique_ptr<RAMLogger> m_ramLogger, and initialize it in Emulator constructor.
Events / System
src/core/system.h
Added GUI event struct PCSX::Events::GUI::RAMFocus { uint32_t address; uint32_t size; } for viewer navigation.
GUI integration
src/gui/gui.h, src/gui/gui.cc
Added Widgets::RAMViewer setting and member; Debug → Memory Editors menu toggle; lifecycle management to enable/disable/upload RAM logger when viewer is shown/hidden.
RAM Viewer widget
src/gui/widgets/ram-viewer.h, src/gui/widgets/ram-viewer.cc
New PCSX::Widgets::RAMViewer: shader-based rendering sampling RAM + read/write/exec heatmaps, decay logic, grid/hex overlays, hover/click interaction (emit JumpToMemory), UI controls (channel toggles, color pickers, decay slider), and focus handling.
Zoomable base
src/gui/widgets/zoomable-image.h, src/gui/widgets/zoomable-image.cc
New ZoomableImage base class with zoom/pan/reset, handlePanZoom helper for mouse wheel & drag; used by viewers.
VRAM viewer refactor
src/gui/widgets/vram-viewer.h, src/gui/widgets/vram-viewer.cc
VRAMViewer now derives from ZoomableImage; removed local pan/zoom state and zoom override; added defaultViewSize() override.
Project files
vsprojects/core/..., vsprojects/gui/...
Added new source/header entries for ramlogger, ram-viewer, and zoomable-image in relevant .vcxproj and .filters files.

Sequence Diagram

sequenceDiagram
    participant CPU as CPU
    participant Debug as Debug
    participant Emulator as Emulator
    participant Logger as RAMLogger
    participant GPU as OpenGL
    participant GUI as GUI
    participant Viewer as RAMViewer

    CPU->>Debug: execute instruction / load/store / DMA
    Debug->>Logger: recordAccess(physAddr, width, type, cycle)
    Logger->>Logger: store cycle timestamps (per-byte)

    Viewer->>GUI: draw() / visible
    GUI->>Emulator: ensure m_ramLogger exists
    Emulator->>Logger: enable()
    Logger->>GPU: create textures (RAM, heatmaps)

    GUI->>Logger: uploadRAM()
    Logger->>GPU: glTexSubImage2D(RAM bytes)
    GUI->>Logger: uploadHeatmaps()
    Logger->>GPU: glTexSubImage2D(heatmap arrays)

    Viewer->>GPU: bind textures & set uniforms
    GPU-->>Viewer: render sampled RAM + heatmaps
    Viewer-->>GUI: hover/click → emit RAMFocus / JumpToMemory
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • Breakpoint improvements #1809 — Modifies Debug/breakpoint logic; likely related where debug DMA/read/write hooks and breakpoint checks intersect with the new RAM logging calls.

Poem

🐇 I hop through bytes both near and far,
I mark their traces — read, write, and spar.
Heatmaps bloom where cycles once played,
Zoom, click, and watch the memory parade.
A rabbit's cheer for every trace displayed!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 3.39% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No description was provided by the author, making it impossible to assess relatedness to the changeset. Add a description explaining the purpose, functionality, and integration of the new RAM viewer feature.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add ram viewer' accurately summarizes the main change: introducing a new RAM viewer widget with supporting logging infrastructure.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (6)
src/core/debug.cc (1)

228-241: Nested ternaries for width could be clearer.

The width calculations work correctly, but the nested ternary expressions reduce readability. Consider extracting to a helper or using a more explicit form.

♻️ Optional: Clearer width calculation
         // RAM logger: record loads and stores
         if (ramLoggerEnabled) {
             PSXAddress loadStoreAddr(normalizeAddress(offset));
             if (loadStoreAddr.type == PSXAddress::Type::RAM) {
                 if (isLoad) {
-                    unsigned width = (isLB || isLBU) ? 1 : (isLH || isLHU) ? 2 : 4;
+                    unsigned width = 4;
+                    if (isLB || isLBU) width = 1;
+                    else if (isLH || isLHU) width = 2;
                     ramLogger->recordAccess(loadStoreAddr.physical, width, RAMLogger::AccessType::Read, cycle);
                 }
                 if (isStore) {
-                    unsigned width = isSB ? 1 : isSH ? 2 : 4;
+                    unsigned width = 4;
+                    if (isSB) width = 1;
+                    else if (isSH) width = 2;
                     ramLogger->recordAccess(loadStoreAddr.physical, width, RAMLogger::AccessType::Write, cycle);
                 }
             }
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/debug.cc` around lines 228 - 241, The nested ternary expressions for
computing access width reduce readability in the RAM logging block; replace them
with a small helper (e.g., computeLoadWidth/computeStoreWidth or a single
computeAccessWidth) and call it where ramLogger->recordAccess is invoked inside
the ramLoggerEnabled branch; reference the existing symbols PSXAddress,
normalizeAddress, isLoad, isLB, isLBU, isLH, isLHU, isStore, isSB, isSH and
ensure the new helper returns unsigned width used by ramLogger->recordAccess
with RAMLogger::AccessType::Read/Write and cycle.
src/core/ramlogger.h (2)

64-66: Note: Significant memory footprint.

The three timestamp arrays consume 96MB total (3 × 8MB × 4 bytes). This is inherent to the per-byte tracking design. Consider documenting this memory requirement or lazily allocating these arrays only when the logger is first enabled.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/ramlogger.h` around lines 64 - 66, The three fixed-size arrays
m_readTimestamps, m_writeTimestamps, and m_execTimestamps allocate ~96MB at
static construction causing a large memory footprint; change them from static
arrays to lazily allocated containers (e.g. std::vector<uint32_t> or
std::unique_ptr<uint32_t[]>) initialized empty and allocate/reserve c_maxBytes
only when the RAM logger is first enabled (e.g. in RamLogger::enable or
initialize method), and free or clear them on disable; alternatively, if you
prefer not to change allocation strategy, add a clear comment above these
members documenting the ~96MB requirement so callers are aware.

22-23: Prefer <cstring> over <string.h> for C++.

While both work, <cstring> is the C++ header and places memset in the std namespace.

Suggested fix
 `#include` <stdint.h>
-#include <string.h>
+#include <cstdint>
+#include <cstring>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/ramlogger.h` around lines 22 - 23, Replace the C header include
"<string.h>" with the C++ header "<cstring>" in the includes at the top of
ramlogger.h and update any uses of C string functions (e.g., memset) in
functions or methods within the file (look for memset calls) to their std::
namespace equivalents (e.g., std::memset) so the code uses the C++ standard
headers/namespaces consistently.
src/core/ramlogger.cc (1)

40-75: Potential resource leak on partial initialization failure.

If texture creation partially succeeds (e.g., read and write heatmaps created but exec fails), the early return leaves textures allocated but m_hasResources remains false. A subsequent enable() call would attempt to recreate all textures without first cleaning up the existing ones.

Consider either cleaning up on failure or checking exists() before create().

Suggested defensive approach
+    // Clean up any partial state first
+    if (m_readHeatmapTex.exists()) m_readHeatmapTex.destroy();
+    if (m_writeHeatmapTex.exists()) m_writeHeatmapTex.destroy();
+    if (m_execHeatmapTex.exists()) m_execHeatmapTex.destroy();
+    if (m_ramTexture.exists()) m_ramTexture.destroy();
+
     // Create heatmap textures (GL_R32UI - unsigned integer)
     m_readHeatmapTex.create(c_width, c_maxHeight, GL_R32UI);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/ramlogger.cc` around lines 40 - 75, enable() can leave
partially-created OpenGL textures allocated on early return, causing resource
leaks because m_hasResources stays false; modify enable() to either check
texture exists() before create() or free any textures created on failure.
Specifically, after each .create(...) and exists() check for m_readHeatmapTex,
m_writeHeatmapTex, m_execHeatmapTex and m_ramTexture, ensure that on failure you
call the corresponding cleanup/destroy method (or a helper like
releaseResources()) to delete any previously-created textures and reset state,
or change flow to skip create() if exists() is true so repeated enable() calls
are idempotent; update m_hasResources only after all resources are successfully
created.
src/gui/widgets/vram-viewer.cc (1)

726-735: moveTo() shadows the base class method with different semantics.

This VRAMViewer::moveTo() takes UV-space coordinates and translates them differently than ZoomableImage::moveTo(), which takes pixel positions. While this works, it could cause confusion since the base class method is hidden. Consider renaming this to something more descriptive like moveToUV() or centerOnVRAMPos() to make the semantic difference explicit.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gui/widgets/vram-viewer.cc` around lines 726 - 735, The
VRAMViewer::moveTo method uses UV-space semantics that differ from
ZoomableImage::moveTo (pixel-space); rename VRAMViewer::moveTo to a descriptive
name like moveToUV or centerOnVRAMPos, update its declaration in the class and
all call sites to the new name, and remove/hide any accidental override so the
base-class ZoomableImage::moveTo remains unshadowed; ensure any tests or
references (e.g., calls in UI code or signal handlers) are updated to call the
new method name and keep the original implementation body unchanged.
src/gui/widgets/ram-viewer.h (1)

22-22: Unused <string> include.

The <string> header doesn't appear to be used directly in this header file. Consider removing it if not needed.

Suggested fix
 `#pragma` once

-#include <string>
-
 `#include` "GL/gl3w.h"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gui/widgets/ram-viewer.h` at line 22, The header includes an unused
<string> header; remove the `#include` <string> line from
src/gui/widgets/ram-viewer.h and rebuild, and if any declarations in the
RamViewer class (or related symbols in this header) rely on std::string move
that dependency to the implementation file (ram-viewer.cpp) or forward-declare
as needed so the header no longer requires <string>.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/gui/widgets/ram-viewer.cc`:
- Around line 209-214: In PCSX::Widgets::RAMViewer::focusOn, the address is
masked with 0x1fffff unconditionally then optionally with 0x7fffff, which
truncates addresses for 8MB mode; change the logic so the mask is chosen based
on Emulator::Setting8MB (apply 0x7fffff when
g_emulator->settings.get<Emulator::Setting8MB>() is true, otherwise apply
0x1fffff) so the address is correctly converted to a physical RAM offset.

---

Nitpick comments:
In `@src/core/debug.cc`:
- Around line 228-241: The nested ternary expressions for computing access width
reduce readability in the RAM logging block; replace them with a small helper
(e.g., computeLoadWidth/computeStoreWidth or a single computeAccessWidth) and
call it where ramLogger->recordAccess is invoked inside the ramLoggerEnabled
branch; reference the existing symbols PSXAddress, normalizeAddress, isLoad,
isLB, isLBU, isLH, isLHU, isStore, isSB, isSH and ensure the new helper returns
unsigned width used by ramLogger->recordAccess with
RAMLogger::AccessType::Read/Write and cycle.

In `@src/core/ramlogger.cc`:
- Around line 40-75: enable() can leave partially-created OpenGL textures
allocated on early return, causing resource leaks because m_hasResources stays
false; modify enable() to either check texture exists() before create() or free
any textures created on failure. Specifically, after each .create(...) and
exists() check for m_readHeatmapTex, m_writeHeatmapTex, m_execHeatmapTex and
m_ramTexture, ensure that on failure you call the corresponding cleanup/destroy
method (or a helper like releaseResources()) to delete any previously-created
textures and reset state, or change flow to skip create() if exists() is true so
repeated enable() calls are idempotent; update m_hasResources only after all
resources are successfully created.

In `@src/core/ramlogger.h`:
- Around line 64-66: The three fixed-size arrays m_readTimestamps,
m_writeTimestamps, and m_execTimestamps allocate ~96MB at static construction
causing a large memory footprint; change them from static arrays to lazily
allocated containers (e.g. std::vector<uint32_t> or std::unique_ptr<uint32_t[]>)
initialized empty and allocate/reserve c_maxBytes only when the RAM logger is
first enabled (e.g. in RamLogger::enable or initialize method), and free or
clear them on disable; alternatively, if you prefer not to change allocation
strategy, add a clear comment above these members documenting the ~96MB
requirement so callers are aware.
- Around line 22-23: Replace the C header include "<string.h>" with the C++
header "<cstring>" in the includes at the top of ramlogger.h and update any uses
of C string functions (e.g., memset) in functions or methods within the file
(look for memset calls) to their std:: namespace equivalents (e.g., std::memset)
so the code uses the C++ standard headers/namespaces consistently.

In `@src/gui/widgets/ram-viewer.h`:
- Line 22: The header includes an unused <string> header; remove the `#include`
<string> line from src/gui/widgets/ram-viewer.h and rebuild, and if any
declarations in the RamViewer class (or related symbols in this header) rely on
std::string move that dependency to the implementation file (ram-viewer.cpp) or
forward-declare as needed so the header no longer requires <string>.

In `@src/gui/widgets/vram-viewer.cc`:
- Around line 726-735: The VRAMViewer::moveTo method uses UV-space semantics
that differ from ZoomableImage::moveTo (pixel-space); rename VRAMViewer::moveTo
to a descriptive name like moveToUV or centerOnVRAMPos, update its declaration
in the class and all call sites to the new name, and remove/hide any accidental
override so the base-class ZoomableImage::moveTo remains unshadowed; ensure any
tests or references (e.g., calls in UI code or signal handlers) are updated to
call the new method name and keep the original implementation body unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a28c4711-8395-421e-87e4-70f30cab92a7

📥 Commits

Reviewing files that changed from the base of the PR and between 2797eab and 9546242.

📒 Files selected for processing (15)
  • src/core/debug.cc
  • src/core/debug.h
  • src/core/psxemulator.cc
  • src/core/psxemulator.h
  • src/core/ramlogger.cc
  • src/core/ramlogger.h
  • src/core/system.h
  • src/gui/gui.cc
  • src/gui/gui.h
  • src/gui/widgets/ram-viewer.cc
  • src/gui/widgets/ram-viewer.h
  • src/gui/widgets/vram-viewer.cc
  • src/gui/widgets/vram-viewer.h
  • src/gui/widgets/zoomable-image.cc
  • src/gui/widgets/zoomable-image.h

Comment thread src/gui/widgets/ram-viewer.cc
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/gui/widgets/ram-viewer.cc`:
- Around line 218-223: The computed endAddr can overflow when doing uint32_t
endAddr = address + size; — clamp/saturate endAddr to the RAM size limit before
converting to coordinates: compute endAddr = std::min(address + size, RAM_SIZE)
(or equivalent saturating logic) and then compute x1, y1, x2, y2 from address
and the clamped endAddr so the focus rectangle never wraps; update the logic
around the existing endAddr/address/size -> x1,y1,x2,y2 conversion to use the
clamped value.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 70457977-8156-44b1-9670-93c283af8b1f

📥 Commits

Reviewing files that changed from the base of the PR and between 9546242 and aa2113e.

📒 Files selected for processing (1)
  • src/gui/widgets/ram-viewer.cc

Comment on lines +218 to +223
uint32_t endAddr = address + size;
float x1 = float(address % 2048);
float y1 = float(address / 2048);
float x2 = float(endAddr % 2048);
float y2 = float(endAddr / 2048);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Clamp endAddr to avoid overflow/wrap in focus range math.

uint32_t endAddr = address + size; can wrap and generate an invalid focus rectangle for large size. Saturate to RAM size before converting to coordinates.

Proposed fix
 void PCSX::Widgets::RAMViewer::focusOn(uint32_t address, uint32_t size) {
     // Convert PS1 address to physical RAM offset
-    if (g_emulator->settings.get<Emulator::Setting8MB>()) {
+    bool is8MB = g_emulator->settings.get<Emulator::Setting8MB>();
+    if (is8MB) {
         address &= 0x7fffff;
     } else {
         address &= 0x1fffff;
     }

     // Convert to pixel coordinates in the 2048-wide layout
-    uint32_t endAddr = address + size;
+    const uint32_t ramSize = is8MB ? 0x800000u : 0x200000u;
+    uint32_t endAddr = (size > (ramSize - address)) ? ramSize : (address + size);
     float x1 = float(address % 2048);
     float y1 = float(address / 2048);
     float x2 = float(endAddr % 2048);
     float y2 = float(endAddr / 2048);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gui/widgets/ram-viewer.cc` around lines 218 - 223, The computed endAddr
can overflow when doing uint32_t endAddr = address + size; — clamp/saturate
endAddr to the RAM size limit before converting to coordinates: compute endAddr
= std::min(address + size, RAM_SIZE) (or equivalent saturating logic) and then
compute x1, y1, x2, y2 from address and the clamped endAddr so the focus
rectangle never wraps; update the logic around the existing endAddr/address/size
-> x1,y1,x2,y2 conversion to use the clamped value.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
vsprojects/gui/gui.vcxproj.filters (1)

112-113: Assign Visual Studio filters for the newly added widget files.

These entries are currently unfiltered, so they’ll appear outside the existing widget groups in Solution Explorer.

📁 Proposed filter mapping
-    <ClCompile Include="..\..\src\gui\widgets\zoomable-image.cc" />
-    <ClCompile Include="..\..\src\gui\widgets\ram-viewer.cc" />
+    <ClCompile Include="..\..\src\gui\widgets\zoomable-image.cc">
+      <Filter>Source Files\widgets</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\src\gui\widgets\ram-viewer.cc">
+      <Filter>Source Files\widgets</Filter>
+    </ClCompile>
...
-    <ClInclude Include="..\..\src\gui\widgets\zoomable-image.h" />
-    <ClInclude Include="..\..\src\gui\widgets\ram-viewer.h" />
+    <ClInclude Include="..\..\src\gui\widgets\zoomable-image.h">
+      <Filter>Header Files\widgets</Filter>
+    </ClInclude>
+    <ClInclude Include="..\..\src\gui\widgets\ram-viewer.h">
+      <Filter>Header Files\widgets</Filter>
+    </ClInclude>

Also applies to: 215-216

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vsprojects/gui/gui.vcxproj.filters` around lines 112 - 113, The two new
source entries (zoomable-image.cc and ram-viewer.cc) were added to the project
filters but not assigned to a filter, causing them to appear outside the Widgets
group; update the gui.vcxproj.filters so each <ClCompile
Include="..\..\src\gui\widgets\zoomable-image.cc"> and <ClCompile
Include="..\..\src\gui\widgets\ram-viewer.cc"> has an associated <Filter>
element that maps them into the existing widget filter (e.g., the same filter
string used by other widget files), and apply the same change for the duplicate
entries at 215-216 so both files appear under the correct Widgets filter in
Solution Explorer.
vsprojects/core/core.vcxproj.filters (1)

148-148: Assign explicit Visual Studio filters for the new RAM logger files.

Line 148 and Line 301 omit <Filter> while most entries in this file set one explicitly. It won’t break builds, but it makes Solution Explorer grouping inconsistent.

🧩 Suggested tidy-up
-    <ClCompile Include="..\..\src\core\ramlogger.cc" />
+    <ClCompile Include="..\..\src\core\ramlogger.cc">
+      <Filter>Source Files</Filter>
+    </ClCompile>
...
-    <ClInclude Include="..\..\src\core\ramlogger.h" />
+    <ClInclude Include="..\..\src\core\ramlogger.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>

Also applies to: 301-301

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vsprojects/core/core.vcxproj.filters` at line 148, The ClCompile entries for
the new ramlogger.cc lack an explicit <Filter>, causing inconsistent Solution
Explorer grouping; locate the ClCompile element with
Include="..\..\src\core\ramlogger.cc" and add a <Filter> element that matches
the existing Core source-file filter used by other core .cc entries (use the
exact filter string used elsewhere to keep grouping consistent); apply the same
change to both occurrences of the ramlogger.cc ClCompile entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@vsprojects/core/core.vcxproj.filters`:
- Line 148: The ClCompile entries for the new ramlogger.cc lack an explicit
<Filter>, causing inconsistent Solution Explorer grouping; locate the ClCompile
element with Include="..\..\src\core\ramlogger.cc" and add a <Filter> element
that matches the existing Core source-file filter used by other core .cc entries
(use the exact filter string used elsewhere to keep grouping consistent); apply
the same change to both occurrences of the ramlogger.cc ClCompile entry.

In `@vsprojects/gui/gui.vcxproj.filters`:
- Around line 112-113: The two new source entries (zoomable-image.cc and
ram-viewer.cc) were added to the project filters but not assigned to a filter,
causing them to appear outside the Widgets group; update the gui.vcxproj.filters
so each <ClCompile Include="..\..\src\gui\widgets\zoomable-image.cc"> and
<ClCompile Include="..\..\src\gui\widgets\ram-viewer.cc"> has an associated
<Filter> element that maps them into the existing widget filter (e.g., the same
filter string used by other widget files), and apply the same change for the
duplicate entries at 215-216 so both files appear under the correct Widgets
filter in Solution Explorer.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8738b9eb-7b4a-477b-a6b9-a84114a29084

📥 Commits

Reviewing files that changed from the base of the PR and between aa2113e and b9313ed.

📒 Files selected for processing (4)
  • vsprojects/core/core.vcxproj
  • vsprojects/core/core.vcxproj.filters
  • vsprojects/gui/gui.vcxproj
  • vsprojects/gui/gui.vcxproj.filters
✅ Files skipped from review due to trivial changes (1)
  • vsprojects/core/core.vcxproj

@nicolasnoble nicolasnoble merged commit 780153c into grumpycoders:main Apr 15, 2026
18 of 20 checks passed
@nicolasnoble nicolasnoble deleted the ram-viewer branch April 15, 2026 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant