monome / norns

norns is many sound instruments.
http://monome.org
GNU General Public License v3.0
630 stars 145 forks source link

No option parameter overlap #1793

Closed skibu closed 1 month ago

skibu commented 1 month ago

Modified menu/params.lua. If a script was using long names for a option parameter label and value then they could overlap. That looked really quite ugly! So changed the drawing of the selector so that if there would be an overlap it tries to use a smaller font. And if would overlap even with the smaller font, then will shift the value to the right, truncating it, so that there is no overlap. This will work for any script's parameter selector.

Since was changing the font, also updated screen.lua so can get current font attributes. That way can save the attributes, change to a narrower font, and then restore the initial font attributes.

An option parameter with the label "Group:" : small_so_no_overlap

If the value is long then there is ugly overlap: overlap

But with this new software the value is reduced in size by using a narrower font, and clipping if need to: withSoftwareChange

You can test this yourself with this simple script:

-- testLongOptionParam.lua

-- For showing that a parameter selector where the label and value are
-- too long is still handled correctly, and that the label and value
-- do not overlap in an ugly way.

function init()
  params:clear()
  some_groups = {
    "short", 
    "longer group", 
    "now we are talking looong", 
    "now super duper long, truncates but works!"
  }

  params:add_separator("idSep1", "Option Parameter")
  params:add_option("group","Group:", some_groups, 1) 
end
tehn commented 1 month ago

thank you for investigating this.

i would prefer not to add this level of complexity to solve this problem.

i feel it is up to the script author to keep option parameters short as to not overlap. this size-based solution only solves only a narrow range of cases and is prone to becoming very illegible quickly with smaller fonts.

we've been discussing alternate approaches to deal with text overflow and hope to test out a few ideas soon.

i'm going to close this out but we can continue discussing.

skibu commented 1 month ago

Brian, I don’t understand why this implementation wasn’t accepted. The code doesn’t continue to reduce the size of the font to an unreadable level. It only tries a single reduction, to a known legible font. And if that is not enough it truncates the strung to prevent overlap. I found for the script I’m developing it works quite well, and it would work for any other script as well, not a small range of cases. The test script I provided shows how it works.

And expecting the script author to limit the size of the values displayed is not reasonable. The script doesn’t have a way of knowing if there would be overlap or how many characters can actually be displayed. This means the script author has no way to know what works except be trial and error, which is not a solution. Plus sometimes a script will provide parameter choices based on an external system where the script author cannot control the length of the values, which is the case of what I am developing.

So I hope you reconsider and accept this useful improvement, and encourage new people to contribute to this great open-source project.

catfact commented 1 month ago

Actually we do have functions that are supposed to determine text extents...

...And we had discussed I think adding commands for bounded text

skibu commented 1 month ago

Yes, in my proposed code I used the text extent functionality to determine if a narrower font would be useful.

catfact commented 1 month ago

My point is script authors can in fact do the same thing. System could make this easier by checking the extents of each string given at parameter creation time, and issuing a warning (intended for the script author) at that point. Rather than doing that work every time we draw an options param value.

Especially as determining text extents is another operation that is actually asynchronous, and (iirc) blocks the main thread while the drawing thread completes the extents check. One of the big benefits of splitting out the drawing thread is that most drawing operations don't block, so e.g. sequence timing isn't affected by scrolling in the menu. If we start adding lots of blocking commands to frequent operations like menu refresh, we diminish that benefit.

skibu commented 1 month ago

I've decided to create a library where my proposed changes can be tried out. The idea is that one simply includes the library and they will get the various features, without having to change the code in their script. That way script authors will be able to try out the proposed changes to see how well they work. I'll let folks know when its time to kick the tires.

With respect to blocking the main thread, that is a good point. Therefore for my library version I've made it only do the check if the strings are pretty long in the first place. This would mean that the synchronous text_extent() call would only rarely be performed.

catfact commented 1 month ago

nice! thank you!

please do also consider my suggestion that this work should really happen only once, when the options parameter is created. (*) the options parameter table could store truncated labels, font options, scaling values, or etc.

(* or, if option labels should be considered dynamic, in metatable __index or etc.)