Search Unity

  1. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Feedback Cleanup Commented TMP Code

Discussion in 'UGUI & TextMesh Pro' started by Ferazel, Oct 27, 2020.

  1. Ferazel

    Ferazel

    Joined:
    Apr 18, 2010
    Posts:
    516
    There is a lot of commented out code in the TextMesh Pro package. I realize that this codebase is relatively old and has a lot of legacy, but I it makes trying to understand it as an outsider much more difficult when there is commented out methods/code everywhere. Could I request that you do a pass to cleanup/remove the commented out sections? It gets really confusing as someone who is not familiar with the codebase to parse the code to figure out what it is doing and where things are being set.

    For example, I'm trying to find out how the TMP label sets its clipping rect and I find methods like this:

    Code (CSharp):
    1.        
    2. void UpdateMaskRegions()
    3.         {
    4.             // TODO: Figure out a better way to handle adding an offset to the masking region
    5.             // This region is defined by the RectTransform of the GameObject that contains the RectMask2D component.
    6.             /*
    7.             // Update Masking Region
    8.             if (m_TextViewportRectMask != null)
    9.             {
    10.                 Rect viewportRect = m_TextViewportRectMask.canvasRect;
    11.  
    12.                 if (viewportRect != m_CachedViewportRect)
    13.                 {
    14.                     m_CachedViewportRect = viewportRect;
    15.  
    16.                     viewportRect.min -= m_TextViewport.offsetMin * 0.5f;
    17.                     viewportRect.max -= m_TextViewport.offsetMax * 0.5f;
    18.  
    19.                     if (m_CachedInputRenderer != null)
    20.                         m_CachedInputRenderer.EnableRectClipping(viewportRect);
    21.  
    22.                     if (m_TextComponent.canvasRenderer != null)
    23.                         m_TextComponent.canvasRenderer.EnableRectClipping(viewportRect);
    24.  
    25.                     if (m_Placeholder != null && m_Placeholder.enabled)
    26.                         m_Placeholder.canvasRenderer.EnableRectClipping(viewportRect);
    27.                 }
    28.             }
    29.             */
    30.         }
    31.  
    This is just a small chunk of a very large amount of commented out code. I don't mind a commented out log statement here or there, but this is pretty difficult. Is there a reason why you keep all of this code around? Could you not keep these as branches if you need the code around still?
     
    Last edited: Oct 27, 2020
  2. Stephan_B

    Stephan_B

    Joined:
    Feb 26, 2017
    Posts:
    6,595
    Short answer: part legacy, old habits and more importantly time as you all keep me busy with potential issues and feature requested, etc. :)

    I have started to clean up those comments and as well as gradually improving the code summary / documentation. A lot of work has been done on the documentation front which I just need to finish reviewing and include in a future release of the package. Again, just so much to do with so little time.

    In terms of your question, is this in the context of a <TextMeshProUGUI> component or a <TextMeshPro> component?
     
  3. Ferazel

    Ferazel

    Joined:
    Apr 18, 2010
    Posts:
    516
    Yeah I realize it's not urgent compared to bugs/features, but it's just a small annoyance whenever I need to dig through TMP code. ;) So I'm glad you're slowly improving it when you get a chance to. I mainly wanted to be a noisy user about it to see if maybe you could dedicate some time to it.

    In terms of my problem I was trying to solve, it's pretty custom so I needed to look into the code. I have a textured effect that I need to overlay on top of the text in object space to apply an effect to the text as a whole in the fragment shader. TMP only supports UV per character/line/paragraph and always maps the characters from 0->1. I need to set a texcoord of the vertex to be relative to object space that I can convert into a UV. Such that if the label moves in worldspace the UV doesn't change as coordinate was determined in relative objectspace position. Since cliprect does 90% of this already I was digging into maybe passing a clip rect in without enabling clipping. Currently, the need is to only apply this for UGUI labels, but I might need to apply it to mesh labels too.

    My current plan is to make the TMP label use a material instance and send the material a property of the parent's worldspace coordinate so I could derive character's object space positions in the vertex shader that I can convert into an object space UV. Which I can use to do the lookup in the frag shader to apply the text effect. Maybe you have a better solution?