plugins/text-freetype2: Clean up file access code#13467
Open
Lain-B wants to merge 6 commits into
Open
Conversation
This code is Windows-only, where the source is already deprecated.
This improves support for large text file sizes when using log mode and fixes types that were unnecessarily inconsistent.
The `size` parameter of fread should be consistent with the type used with the array you're reading into, while `count` should specify how many of that type you're reading. Considering we're reading an array of characters, `size` should be the size of a `char` (1) and `count` should be the number of characters we want to read.
WizardCM
added
kind/bug
Lain-B
force-pushed
the
fix-freetype-null-chars
branch
from
70d48a4 to
8b33d16
Compare
Log mode would call fread() for single individual characters from end of the file to the beginning in order to check to see where it has to start reading from. This seems unnecessarily unoptimal, so fix this to read 1024 byte chunks and check to see where to start from that way. Should result in far fewer unnecessary fread() calls. (Author note: I would just make this read the whole file, but I am doing it this way under the assumption that there are users who may use this with very large files of log data.)
Files might sometimes contain '\0' characters, which would cause all text after the '\0' characters to not be included. Fixes obsproject#7595
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR contains numerous fixes for file access with the text-freetype2 text source:
Fixes #7595
Motivation and Context
In my journey of investigating #7595 (that many have proposed fixes to over the years), I realized that there was quite a few things to fix with file reading. This is the culmination of those fixes.
There are probably more things that could probably be fixed in text-freetype2 -- I would argue the source needs to be completely rewritten from scratch with a text renderer that also handles text shaping properly -- but I would prefer to keep it focused on just fixing the issue properly, and fixing a few related issues and not like, bike shed the god forsaken text-freetype2 source to death.
If someone wants to rewrite this thing then please do, this thing needs a bulldozer. I just want to get the issues and those other PRs closed to remove some ancient gunk from our repository.
How Has This Been Tested?
Tested on Linux. Tested file reading to ensure that it worked, tested log mode to ensure that it also still worked properly.
Types of changes
Checklist: