Skip to content

Conversation

@GOROman
Copy link
Owner

@GOROman GOROman commented May 19, 2025

Summary

  • improve printing speed by removing temporary String objects

Testing

  • git status --short

@GOROman GOROman requested a review from Copilot May 19, 2025 08:05
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes character rendering by eliminating per-iteration String allocations and using direct char access for display.

  • Introduce a const char* pointer to buffer data and extract each character directly
  • Replace String.substring and canvas.printf calls with canvas.write(ch)
  • Remove temporary String objects in the print loop
Comments suppressed due to low confidence (3)

src/main.cpp:204

  • Consider using buffer.length() (or buffer.size()) instead of the external len variable to ensure you iterate over the actual string length and avoid potential out-of-bounds access.
for (int i = 0; i < len; i++)

src/main.cpp:202

  • [nitpick] The variable name cstr is ambiguous; consider renaming it to something more descriptive, like bufferChars or dataPtr, to clarify its purpose.
const char *cstr = buffer.c_str();

src/main.cpp:201

  • [nitpick] The variable count could be renamed to printedCount or similar to better convey that it tracks the number of characters printed.
int count = 0;

// 1文字づつ表示
String str = buffer.substring(i, i + 1);
char ch = cstr[i];

Copy link

Copilot AI May 19, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding a comment to explain why spaces and question marks are skipped here, improving code readability for future maintainers.

Suggested change
// Skip spaces and question marks to avoid unnecessary sound effects or visual updates.

Copilot uses AI. Check for mistakes.
@GOROman GOROman requested a review from Copilot May 21, 2025 04:18
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes the task_print function by replacing per-character String allocations with direct buffer access and a single-character write call to improve rendering speed.

  • Introduces a const char* pointer to the string data
  • Replaces String.substring with indexing into the C-string
  • Switches from canvas.printf to canvas.write for single-character output
Comments suppressed due to low confidence (3)

src/main.cpp:202

  • [nitpick] The variable name cstr is generic; consider renaming to buffer_cstr or textPtr to improve readability and clarify its relationship to buffer.
const char *cstr = buffer.c_str();

src/main.cpp:206

  • Indexing into a C-string may split multi-byte/UTF-8 characters, leading to rendering errors. Consider using a method that respects character boundaries (e.g., substring(i, i + 1)) or iterate with a Unicode-aware iterator.
char ch = cstr[i];

src/main.cpp:202

  • The loop uses len but it isn’t shown as being set to buffer.length(). Ensure len is defined as size_t len = buffer.length(); after creating buffer to avoid out-of-bounds access.
const char *cstr = buffer.c_str();

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.

2 participants