Skip to content
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

API Proposal: Add safe overloads to TextRenderer to get output strings. #3710

Open
JeremyKuhne opened this issue Aug 10, 2020 · 4 comments
Open
Assignees
Labels
api-approved (4) API was approved in API review, it can be implemented help wanted Good issue for external contributors
Milestone

Comments

@JeremyKuhne
Copy link
Member

JeremyKuhne commented Aug 10, 2020

Proposal

We need to add new overloads that allow getting out the actual rendered string when using ellipses options (TextFormatFlags.PathEllipsis and TextFormatFlags.WordEllipsis). This is currently done with TextFormatFlags.ModifyString, which mutates the input string. We block this option in the newer span overloads and have obsoleted it.

This is a follow-up to #3651.

namespace System.Windows.Forms
{
    public sealed class TextRenderer
    {
        // Existing flags APIs:
        public static void DrawText(IDeviceContext dc, string text, Font font, Point pt, Color foreColor, Color backColor, TextFormatFlags flags);
        public static void DrawText(IDeviceContext dc, string text, Font font, Point pt, Color foreColor, TextFormatFlags flags);
        public static void DrawText(IDeviceContext dc, string text, Font font, Rectangle bounds, Color foreColor, Color backColor, TextFormatFlags flags);
        public static void DrawText(IDeviceContext dc, string text, Font font, Rectangle bounds, Color foreColor, TextFormatFlags flags);
        public static Size MeasureText(IDeviceContext dc, string text, Font font, Size proposedSize, TextFormatFlags flags);
        public static Size MeasureText(string text, Font font, Size proposedSize, TextFormatFlags flags);
        public static void DrawText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Point pt, Color foreColor, Color backColor, TextFormatFlags flags);
        public static void DrawText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Point pt, Color foreColor, TextFormatFlags flags);
        public static void DrawText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Rectangle bounds, Color foreColor, Color backColor, TextFormatFlags flags);
        public static void DrawText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Rectangle bounds, Color foreColor, TextFormatFlags flags);
        public static Size MeasureText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Size proposedSize, TextFormatFlags flags);
        public static Size MeasureText(ReadOnlySpan<char> text, Font font, Size proposedSize, TextFormatFlags flags);

        // Proposed
        public static void DrawText(IDeviceContext dc, string text, Font font, Point pt, Color foreColor, Color backColor, TextFormatFlags flags, out string outputText);
        public static void DrawText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Point pt, Color foreColor, Color backColor, TextFormatFlags flags, Span<char> outputTextBuffer, out int outputTextLength);
        public static void DrawText(IDeviceContext dc, string text, Font font, Rectangle bounds, Color foreColor, Color backColor, TextFormatFlags flags, out string outputText);
        public static void DrawText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Rectangle bounds, Color foreColor, Color backColor, TextFormatFlags flags, Span<char> outputTextBuffer, out int outputTextLength);
        public static Size MeasureText(IDeviceContext dc, string text, Font font, Size proposedSize, TextFormatFlags flags, out string outputText);
        public static Size MeasureText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Size proposedSize, TextFormatFlags flags, Span<char> outputTextBuffer, out int outputTextLength);
        public static Size MeasureText(string text, Font font, Size proposedSize, TextFormatFlags flags, out string outputText);
        public static Size MeasureText(ReadOnlySpan<char> text, Font font, Size proposedSize, TextFormatFlags flags, Span<char> outputTextBuffer, out int outputTextLength);
    }
}

Justification

It is still a valid scenario to want to know what Windows actually outputs. Providing these overloads will let us fully block the mutation of the input strings, which is extremely dangerous.

The proposed surface area aligns with the existing API definitions and allows utilizing the same buffer for the Span APIs for advanced users.

Implementation Details

These APIs will presume you want the output string without requiring the obsoleted flag. When the ellipses flags are set we will get the actual output string from Windows and either create a new string or copy the output into the Span buffer provided and return the length. In all other scenarios we will pass back the input.

The Span overloads will require that the passed in Span<char> is at least as long as the input ReadOnlySpan<char>. If it is not, the API will throw.

We'll skip the overloads for just foreColor and document that Color.Empty is required for a transparent background as these APIs are not as common.

We'll match additional throwing behavior of the original APIs. The original APIs did not throw for null or empty strings, they just no-op.

@JeremyKuhne JeremyKuhne added the api-suggestion (1) Early API idea and discussion, it is NOT ready for implementation label Aug 10, 2020
@weltkante
Copy link
Contributor

Interesting proposal, I like it better than the usual mapping of mutable argument via ref string text and Span<char> text

Having separate input/output parameters

  • caller can still pass the same variable for both input and output if he doesn't need two variables
  • caller can still use out var syntax even in situations where he couldn't use ref-syntax (e.g. on a foreach loop variable)
  • allows caller to input a string (viewed as readonly span) but write output into a span buffer, something which would not be possible with a mutable argument

(Just felt it worth to point out the advantages of separate input/output arguments over providing a single mutable argument because I had to think a bit before I noticed them. Also supporting passing the same span into both input and output may require special coding, so its worth mentioning early that this is a case that makes sense.)

@ghost ghost added 🚧 work in progress Work that is current in progress and removed 🚧 work in progress Work that is current in progress labels Aug 10, 2020
@JeremyKuhne JeremyKuhne reopened this Sep 9, 2020
@JeremyKuhne JeremyKuhne added this to the 6.0 milestone Sep 9, 2020
@JeremyKuhne JeremyKuhne self-assigned this Sep 9, 2020
@JeremyKuhne JeremyKuhne added api-ready-for-review (2) API is ready for formal API review; applied by the issue owner and removed api-suggestion (1) Early API idea and discussion, it is NOT ready for implementation labels Sep 9, 2020
@terrajobst terrajobst added api-approved (4) API was approved in API review, it can be implemented and removed api-ready-for-review (2) API is ready for formal API review; applied by the issue owner labels Sep 15, 2020
@terrajobst
Copy link
Member

terrajobst commented Sep 15, 2020

Video

  • Normally, output buffers go right after the input buffer, but we'll keep it last here to align with existing overloads and (more importantly) the out string
namespace System.Windows.Forms
{
    public sealed class TextRenderer
    {
        // Existing flags APIs:
        // public static void DrawText(IDeviceContext dc, string text, Font font, Point pt, Color foreColor, Color backColor, TextFormatFlags flags);
        // public static void DrawText(IDeviceContext dc, string text, Font font, Point pt, Color foreColor, TextFormatFlags flags);
        // public static void DrawText(IDeviceContext dc, string text, Font font, Rectangle bounds, Color foreColor, Color backColor, TextFormatFlags flags);
        // public static void DrawText(IDeviceContext dc, string text, Font font, Rectangle bounds, Color foreColor, TextFormatFlags flags);
        // public static Size MeasureText(IDeviceContext dc, string text, Font font, Size proposedSize, TextFormatFlags flags);
        // public static Size MeasureText(string text, Font font, Size proposedSize, TextFormatFlags flags);
        // public static void DrawText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Point pt, Color foreColor, Color backColor, TextFormatFlags flags);
        // public static void DrawText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Point pt, Color foreColor, TextFormatFlags flags);
        // public static void DrawText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Rectangle bounds, Color foreColor, Color backColor, TextFormatFlags flags);
        // public static void DrawText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Rectangle bounds, Color foreColor, TextFormatFlags flags);
        // public static Size MeasureText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Size proposedSize, TextFormatFlags flags);
        // public static Size MeasureText(ReadOnlySpan<char> text, Font font, Size proposedSize, TextFormatFlags flags);

        public static void DrawText(IDeviceContext dc, string text, Font font, Point pt, Color foreColor, Color backColor, TextFormatFlags flags, out string outputText);
        public static void DrawText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Point pt, Color foreColor, Color backColor, TextFormatFlags flags, Span<char> outputTextBuffer, out int outputTextLength);
        public static void DrawText(IDeviceContext dc, string text, Font font, Rectangle bounds, Color foreColor, Color backColor, TextFormatFlags flags, out string outputText);
        public static void DrawText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Rectangle bounds, Color foreColor, Color backColor, TextFormatFlags flags, Span<char> outputTextBuffer, out int outputTextLength);
        public static Size MeasureText(IDeviceContext dc, string text, Font font, Size proposedSize, TextFormatFlags flags, out string outputText);
        public static Size MeasureText(IDeviceContext dc, ReadOnlySpan<char> text, Font font, Size proposedSize, TextFormatFlags flags, Span<char> outputTextBuffer, out int outputTextLength);
        public static Size MeasureText(string text, Font font, Size proposedSize, TextFormatFlags flags, out string outputText);
        public static Size MeasureText(ReadOnlySpan<char> text, Font font, Size proposedSize, TextFormatFlags flags, Span<char> outputTextBuffer, out int outputTextLength);
    }
}

@IanKemp
Copy link

IanKemp commented Sep 19, 2020

I don't understand why TextFormatFlags.ModifyString was EVER added, let alone implemented... it breaks pretty much every .NET convention. Glad to see this particular piece of cancer is finally being excised, my only question is when do you intend to make using this flag an error? .NET 6?

@RussKie
Copy link
Member

RussKie commented Sep 21, 2020

my only question is when do you intend to make using this flag an error? .NET 6?

We have marked it as obsolete in 5.0 RC1 #3711. As to when this becomes an error - it hasn't been discussed yet.

@RussKie RussKie added the help wanted Good issue for external contributors label Sep 21, 2020
@RussKie RussKie modified the milestones: 6.0, 7.0 Aug 27, 2021
@dreddy-work dreddy-work modified the milestones: .NET 7.0, .NET 8.0 Aug 15, 2022
@merriemcgaw merriemcgaw modified the milestones: .NET 8.0, Future Feb 22, 2023
@merriemcgaw merriemcgaw modified the milestones: Future, .NET 9.0 Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved (4) API was approved in API review, it can be implemented help wanted Good issue for external contributors
Projects
None yet
Development

No branches or pull requests

8 participants