Skip to content

Update mactext-canvas.cpp #6506

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

Closed
wants to merge 2 commits into from
Closed

Conversation

dasbidyendu
Copy link

ENGINE: Ensure line continuations are valid before appending

  • Added checks to ensure line continuations are not empty before
    appending them to the current line’s word continuation.
  • Added comments to clarify previous code.
  • Ensured that chunks are appended only if they are within bounds.
  • Assigned newChunk as a pointer to the current chunk to maintain
    consistency.

Added some checks to make sure The line continuations are not empty before adding them to the current line's word continuation to ensure consistency.

Also added some comments on previous code.

Made sure to append the chunks if it is within bounds.

Added the newChunk as a pointer to the current chunk.
@sev-
Copy link
Member

sev- commented Mar 24, 2025

You must follow our Commit Guidelines: https://wiki.scummvm.org/index.php?title=Commit_Guidelines

And while your code formatting is fine, could you please remove those excessive empty lines around practically every code change? Just follow the code formatting in the rest of the file and do not introduce your own style.

@dasbidyendu
Copy link
Author

Ok i will keep that in mind and make sure to remove unnecessary empty lines

@sev- sev- added the GSoC Part of a Google Summer of Code project label Mar 26, 2025
Comment on lines +87 to +91
for (int i = 0; i < (int)text.size(); i++) {

D(9, "Line Continuations [%d] : %d", i, lineContinuations[i]);

}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for (int i = 0; i < (int)text.size(); i++) {
D(9, "Line Continuations [%d] : %d", i, lineContinuations[i]);
}
for (int i = 0; i < (int)text.size(); i++) {
D(9, "Line Continuations [%d] : %d", i, lineContinuations[i]);
}

Comment on lines 105 to 109
for (int i = 0; i < (int)text.size(); i++) {

D(9, "** chopChunk result %d \"%s\"", i, toPrintable(text[i].encode()).c_str());

}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for (int i = 0; i < (int)text.size(); i++) {
D(9, "** chopChunk result %d \"%s\"", i, toPrintable(text[i].encode()).c_str());
}
for (int i = 0; i < (int)text.size(); i++) {
D(9, "** chopChunk result %d \"%s\"", i, toPrintable(text[i].encode()).c_str());
}

Comment on lines +113 to 120
//Ensure line continuations is valid before accesing index 0
if (!lineContinuations.empty()) {

_text[curLine].wordContinuation = lineContinuations[0];

}


Copy link
Member

@bluegr bluegr Mar 29, 2025

Choose a reason for hiding this comment

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

Suggested change
//Ensure line continuations is valid before accesing index 0
if (!lineContinuations.empty()) {
_text[curLine].wordContinuation = lineContinuations[0];
}
// Ensure line continuations is valid before accessing index 0
if (!lineContinuations.empty()) {
_text[curLine].wordContinuation = lineContinuations[0];
}

Copy link
Contributor

Choose a reason for hiding this comment

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

accessing*

Copy link
Member

Choose a reason for hiding this comment

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

accessing*

Indeed! Updated the change above accordingly :)

Comment on lines +131 to +133
MacFontRun newchunk = *chunk;


Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
MacFontRun newchunk = *chunk;
MacFontRun newchunk = *chunk;

chunk->text += str;

if (w < maxWidth) {
chunk->text += str; //Only append if within bounds
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
chunk->text += str; //Only append if within bounds
chunk->text += str; // Only append if within bounds

… restoration issue in getmaxwidth.

- Check for null pointer before handling any operations in getChunkNum function.
- Check for empty chunks to avoid extra operations.
- avoid signed/unsigned issue in chunk position.
- avoid possible overflow in cursor restoration logic
@dasbidyendu dasbidyendu closed this Apr 5, 2025
@sev-
Copy link
Member

sev- commented Apr 5, 2025

And why this PR was closed?

@dasbidyendu
Copy link
Author

Hey I just closed the PR by mistake and reopened another one , can you checked that one out . I misclicked to close this one out.

@sev-
Copy link
Member

sev- commented Apr 5, 2025

We are generally annoyed by closed reports. It generates too much noise for the whole team.

@dasbidyendu
Copy link
Author

I am sorry I didn’t know that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GSoC Part of a Google Summer of Code project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants