Skip to content

feat(UI): add shader cache duration#1408

Merged
alandtse merged 1 commit into
community-shaders:devfrom
Ungeziefi:Shader-cache-duration
Aug 23, 2025
Merged

feat(UI): add shader cache duration#1408
alandtse merged 1 commit into
community-shaders:devfrom
Ungeziefi:Shader-cache-duration

Conversation

@Ungeziefi
Copy link
Copy Markdown
Contributor

@Ungeziefi Ungeziefi commented Aug 17, 2025

Added an a_elapsedOnly argument to GetStatsString to easily allow the addition of the feature without doing any formatting on the original one to remove the estimated part. Also improved wording for a tooltip.

Added an a_elapsedOnly argument to GetStatsString to easily allow the addition of the feature without doing any formatting on the original one to remove the estimated part. Also improved wording for a tooltip.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 17, 2025

Walkthrough

Updated shader stats formatting to support an “elapsed-only” mode by adding a new optional parameter across ShaderCache APIs and implementations. Adjusted UI to display the last shader cache build duration when tasks exist and refined the Shaders tab tooltip text. No other logic paths were changed.

Changes

Cohort / File(s) Summary
UI: Settings Tab updates
src/Menu/SettingsTabRenderer.cpp
Revised tooltip text for Shaders tab. Added conditional line to render “Last shader cache build duration” using shaderCache->GetShaderStatsString(true, true).
Shader stats API (impl)
src/ShaderCache.cpp
Extended GetShaderStatsString and CompilationSet::GetStatsString signatures to include a_elapsedOnly. Implemented formatting branch to return elapsed-only time when requested; preserved existing behaviors otherwise.
Shader stats API (header)
src/ShaderCache.h
Added optional a_elapsedOnly parameter with defaults to CompilationSet::GetStatsString and ShaderCache::GetShaderStatsString, maintaining backward compatibility for existing calls.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant UI as SettingsTabRenderer
  participant Cache as ShaderCache
  participant Set as CompilationSet

  User->>UI: Open Shaders tab
  UI->>Cache: GetShaderStatsString(a_timeOnly=true, a_elapsedOnly=true)
  Cache->>Set: GetStatsString(true, true)
  Set-->>Cache: "elapsed time"
  Cache-->>UI: "elapsed time"
  UI-->>User: Display "Last shader cache build duration: <elapsed>"
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • alandtse

Poem

I hop through code with caching cheer,
Ticking timers crisp and clear.
Elapsed alone, the numbers sing—
A swift report on shader spring.
Tooltips trimmed, the facts in sight,
I thump my paw: “Build’s alright!” 🐇⏱️

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Copy Markdown

Using provided base ref: bd57c65
Using base ref: bd57c65
Base commit date: 2025-08-17T19:32:46+01:00 (Sunday, August 17, 2025 07:32 PM)
No actionable suggestions for changed features.

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: 0

🧹 Nitpick comments (5)
src/ShaderCache.h (2)

263-263: API extension looks good; consider clarifying param semantics inline.

Defaults keep existing call sites source-compatible. a_elapsedOnly is only meaningful when a_timeOnly is true; documenting that at the declaration helps avoid misuse.

Apply this diff to add a brief note:

-		std::string GetStatsString(bool a_timeOnly = false, bool a_elapsedOnly = false);
+		std::string GetStatsString(bool a_timeOnly = false, bool a_elapsedOnly = false); // when a_timeOnly is true, a_elapsedOnly controls whether to omit ETA

399-399: Forward declaration updated correctly; add a short note mirroring CompilationSet.

Same reasoning as above; tiny comment improves self-documentation at the API surface.

-		std::string GetShaderStatsString(bool a_timeOnly = false, bool a_elapsedOnly = false);
+		std::string GetShaderStatsString(bool a_timeOnly = false, bool a_elapsedOnly = false); // if a_timeOnly, a_elapsedOnly toggles ETA
src/Menu/SettingsTabRenderer.cpp (2)

41-41: Tooltip wording nit: describe both enabled and disabled states.

Clearer phrasing reduces ambiguity when hovering the “Enable Disk Cache” option.

-			ImGui::Text("Disables loading shaders from disk and prevents saving compiled shaders to disk cache.");
+			ImGui::Text("When enabled: loads shaders from disk and saves compiled shaders to the disk cache.\nWhen disabled: prevents both loading from and saving to the disk cache.");

52-55: Prefer showing duration only after at least one completion; also avoid c_str() on a temporary.

  • Using GetCompletedTasks() avoids showing 00:00:00 before any task completes.
  • Storing the stats string is safer and slightly clearer.
-		if (shaderCache->GetTotalTasks() > 0) {
-			ImGui::Text("Last shader cache build duration: %s",
-				shaderCache->GetShaderStatsString(true, true).c_str());
-		}
+		if (shaderCache->GetCompletedTasks() > 0) {
+			const std::string stats = shaderCache->GetShaderStatsString(true, true);
+			ImGui::Text("Last shader cache build duration: %s", stats.c_str());
+		}
src/ShaderCache.cpp (1)

2617-2640: Elapsed-only mode implemented as intended; minor clarity suggestion.

Logic matches the PR intent: elapsed-only when both flags are true; elapsed+ETA otherwise; detailed stats when a_timeOnly is false. Consider noting in the header that a_elapsedOnly is ignored unless a_timeOnly is true (see suggested header comments).

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between bd57c65 and 22c6e2c.

📒 Files selected for processing (3)
  • src/Menu/SettingsTabRenderer.cpp (2 hunks)
  • src/ShaderCache.cpp (2 hunks)
  • src/ShaderCache.h (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

Do not include TODO/FIXME placeholders; provide complete, working solutions

Files:

  • src/Menu/SettingsTabRenderer.cpp
  • src/ShaderCache.h
  • src/ShaderCache.cpp
src/**/*.{cpp,cxx,cc,h,hpp,hxx}

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code

Files:

  • src/Menu/SettingsTabRenderer.cpp
  • src/ShaderCache.h
  • src/ShaderCache.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build plugin and addons
🔇 Additional comments (1)
src/ShaderCache.cpp (1)

2060-2063: LGTM: API forwards the new flag correctly.

The new GetShaderStatsString overload cleanly delegates both flags to the underlying formatter.

@github-actions
Copy link
Copy Markdown

✅ A pre-release build is available for this PR:
Download

@alandtse
Copy link
Copy Markdown
Collaborator

@coderabbitai please provide an appropriate conventional commit given user impact.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 23, 2025

Tip

For best results, initiate chat on the files or code changes.

@alandtse Based on the PR changes, here's an appropriate conventional commit message given the user impact:

feat: add shader cache duration indicator to UI

- Display "Last shader cache build duration" when shaders are compiling
- Show elapsed time to provide user feedback on compilation progress  
- Improve shader cache tooltip clarity for better user understanding

This follows the conventional commit format with feat: since it introduces new user-facing functionality that enhances the user experience by providing visual feedback about shader compilation duration and improves UI clarity.

@alandtse
Copy link
Copy Markdown
Collaborator

@coderabbitai I thought this PR was adding a toggle to a function. Is the indicator actually new for the user?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 23, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Let's examine the actual changes in SettingsTabRenderer.cpp to see if the duration indicator is new
rg -A 10 -B 10 "Last shader cache build duration" src/Menu/SettingsTabRenderer.cpp

# Also check if there were any existing shader duration or stats displays
rg -A 5 -B 5 "GetShaderStatsString\|shader.*duration\|cache.*duration" src/Menu/SettingsTabRenderer.cpp

Length of output: 336


🏁 Script executed:

#!/bin/bash
# Find files related to settings, menu, or rendering
fd -t f -e cpp -e h | rg -i "settings\|menu\|tab\|render" | head -10

# Search for files that might contain shader cache UI code
rg -l "GetShaderStatsString\|shader.*cache\|cache.*duration" --type cpp --type h

# Also search for the specific text mentioned in the summary
rg -l "Last shader cache build duration" --type cpp --type h

Length of output: 514


🏁 Script executed:

#!/bin/bash
# Get basic repository structure
ls -la

# Find all cpp and h files
find . -name "*.cpp" -o -name "*.h" | head -20

# Look for any files containing GetShaderStatsString
find . -name "*.cpp" -o -name "*.h" | xargs grep -l "GetShaderStatsString" 2>/dev/null || echo "No matches found"

Length of output: 513


🏁 Script executed:

#!/bin/bash
# Check what's in the features directory
ls -la features/

# Look for any src directories
find . -type d -name "src" 2>/dev/null

# Check if there are any C++ files anywhere in the repo
find . -name "*.cpp" 2>/dev/null | head -5
find . -name "*.h" 2>/dev/null | head -5

# Check git status to see what repository this is
git remote -v

Length of output: 1037


🏁 Script executed:

#!/bin/bash
# Check current branch and available branches
git branch -a

# Check if this is the PR branch
git status

# Look for any commits related to the PR
git log --oneline -10

# Check if there are any recent commits about shader cache
git log --oneline --grep="shader" -5
git log --oneline --grep="cache" -5

Length of output: 256


@alandtse You're right to question this! Let me clarify based on the information available:

Looking at the AI summary more carefully, this is actually a new user-facing feature. The key evidence:

  1. New UI element: The summary specifically states "Added a UI line in RenderShadersTab that, when shaderCache->GetTotalTasks() > 0, renders 'Last shader cache build duration: '"

  2. The function changes support the new UI: The a_elapsedOnly parameter addition enables this new UI element to show just the elapsed time (without estimated completion time) for this specific display

  3. Conditional display: The duration indicator only appears when shaderCache->GetTotalTasks() > 0, meaning it shows up during active shader compilation

So you're correct that there's a function parameter addition (the toggle), but that toggle specifically enables a new user-facing shader cache duration indicator in the UI. The conventional commit I suggested is appropriate - this is indeed a feat: that adds new user-visible functionality.

The function signature changes are the implementation detail that enables the new feature, but the user impact is the new duration indicator they'll see during shader compilation.

@alandtse alandtse changed the title feat: shader cache duration indicator feat: add shader cache duration indicator Aug 23, 2025
@alandtse alandtse changed the title feat: add shader cache duration indicator feat: add shader cache duration Aug 23, 2025
@alandtse alandtse changed the title feat: add shader cache duration feat(UI): add shader cache duration Aug 23, 2025
@alandtse alandtse merged commit d61defa into community-shaders:dev Aug 23, 2025
23 checks passed
sicsix pushed a commit to sicsix/skyrim-community-shaders that referenced this pull request Sep 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants