Possible CharacterMap enhancements (easy)

[humdinger]

This is hrev29833.

A few possible enhancements:

(done) 1. If I scroll down in the character map on the right side, it'd be nice if the current position was reflected on the left "contents" view by updating the selected block "heading".
(done) 2. By moving the font selection into a drop down menu (to the right of the font size?) it would always be visible what font you're examining.
(wontfix) 3. Is the menu item "Show private blocks" really necessary, is there a need to hide those?

4. Blocks that don't contain any characters could be greyed out (in the "contents" list to the left).
   
5. Left-clicking a character could increase its size (x2 ?) for detailed inspection.
   

(done) 6. The selected character could also be shown black on white in front of the "Code:" line at the button. This way you can be sure you didn't accidentally move the mouse to the adjacent character while taking your eyes from it.
(done) 7. CharacterMap should node-monitor for updated fonts.

[axeld]

1. Indeed, that would be nice. I'm just too lazy to implement that, though :-)

2. Primarily, CharacterMap is a Unicode character map viewer, not a font inspector, therefore I don't think it's overly important to always see that. Especially once the app_server will support font overlays (ie. that means it will substitute the font to get the character). Although I would have an option to turn off those overlays to really only see the contents of the current font, so maybe it's not that a bad idea...

3. That's a font issue: a font might define private characters, and you are able to see them this way. And yes, it makes sense to hide them by default as this doubles the number of characters, and that makes scrolling harder.

4. This is currently not supported by Haiku (ie. BFont::Blocks() is not yet implemented).

5. Too much effort for little gain, I think.

6. Done.

7. It should do so, yes. Maybe it should just use a lazily built font menu instead.

[humdinger]

Thanks for looking into things.

WRT 2.), me as an end-user, I'd use the app to find nice/weird characters. So seeing at which font I'm looking would be beneficial.
I guess that it's not possible to have the font type also been supplied when drag&dropping a character into an editor? Provided the editor checks for these things.
That's an area that I'd find very interesting: intelligent drag&drop (as flattened BMessage?). Provide all kinds of information, e.g. size, color, type alongside the "normal" data of the character. This of course for all kinds of inter-app data transfer. So you'd have ID3 Tags for mp3s, width/height for images, etc.

[axeld]

You might be right about the font, but well. Maybe someone wants to do that; I don't think it's really needed.

Adding the font information to the text would be possible to do. I guess I could change that when I find the time.

[humdinger]

I have one more thing to add, though I don't know if that's how the app is normally used:

8. A drag&dropped character into CharacterMap could scroll to and highlight the according character in the map. You may see a character and be interested in it's mapping, right?

I dunno, your call. As always. :)

[cyberguijarro]

Hi! this sounds quite boring but actually a nice task to start getting involved in the project.

Let me see what I can do ;)

[cyberguijarro]

Second enhancement (font list as a dropdown menu).

[aldeck]

I tested the patch (even though i had to manually apply the parts that are now using translated strings). I've discussed the resulting change with humdinger and noticed it wasn't too good ui wise. A solution would be to keep the original font menu where it is and simply display the current font as text, replacing the "Font size:" label which doesn't add interesting information.

[dsizzle]

Hi, I'm looking into the items as a first crack at Haiku. It's been a while since I programmed in C++ but I'm looking forward to getting back to it a bit.

While testing I noticed something else to add:

9. The minimum window width should be increased. Currently at minimum width, the left/right side panes jump distractingly as different characters are selected (due to the width of the Code/UTF-8 data)

[dsizzle]

Patch for Enhancement 1

[dsizzle]

seems like Enhancement 8 is already in the code, but I'm not sure how to test it.

[pulkomandy]

Hi,

Patch 1: In the (unlikely) "character not mapped" case, shouldn't SelectblockForCharacter return, rather than break? What happens if you call Select with an invalid block number? If this is done on purpose (didn't check the code, do we have an "unmapped" block at the end of the list?), a comment should be added to remember people reading the code of that. Something like this:

// Character is not mapped, we will scroll to the special "unmapped" block at the end of the list

Patch 2: It looks like the code for handling displayName would be much simpler if you used a BString instead of a char*. I think the " > " is not needed (we use "DejaVu Sans Book" elsewhere, with "DejaVu Sans" the family and "Book" the style).

Thanks for the patches!

[dsizzle]

rewrote C-style string operations for displayName to use BString - apply *after* 3651-Enh2.patch

[dsizzle]

You're welcome for the patches! Thanks for reviewing them pulkomandy.

For Patch 1, the break; actually will exit the for loop, and then fall out of the function SelectBlockForCharacter without calling the select command. It is a stylistic difference, but if prefer a return; then I'm happy to change it.

For Patch 2, I rewrote the C-style string lines to use BString...much much nicer! I don't know how to combine patches so I uploaded a second patch.

I also added a patch for Enhancement 7 - node monitoring font directories to rebuild the font menu on the fly. It monitors B_SYSTEM_FONTS_DIRECTORY and B_USER_FONTS_DIRECTORY, which seemed correct but the system font dir is read-only, and the user font dir has a slightly different path (that doesn't exist in my installation). Are those appropriate directories to monitor?

[dsizzle]

dang, I accidentally left a debug printf in line 330 of 3651-Enh7.patch ...should I make a patch for that too?

[dsizzle]

I added a patch for Enhancement 9, setting minimum width of character view pane so it doesn't jump when the code line gets too long. This problem can be seen if you shrink the window horizontally, and then navigate to higher code blocks like Domino tiles. In this patch I included the removal of the debug print accidentally left in patch 3651-Enh7; I hope that's ok.

[pulkomandy]

Hi, You can use git rebase -i to edit your git history and merge or split patches. This would be cleaner and would make them easier to review.

Patch 1 applied in hrev48371.

I took another look at patch 2 and I see another strange thing : you have added a call to fFontSizeSlider->SetLabel(displayName); right after creating the slider with that name already, why is that needed?

On patch 7: unfortunately the list of directories to monitor is a bit bigger. You should use the newly introduced BPathFinder class to get the directories (headers/os/storage/PathFinder.h).

Patch 9: this assumes "w" is the largest character, which may be true only for latin fonts. I'm not sure there is a better solution, however. Also the patch now depends on patch 7 so I can't apply it.

[dsizzle]

Thanks, I'll look into git rebase.

patch 2: You're right, that fFontSizeSlider->SetLabel(displayName); is not needed. I think it's a copy/paste error.

patch 7: Thanks, I will have a look at BPathFinder!

If i can figure out the rebasing I will redo patch 2 to remove the extra line and merge the 2 patches, as well as fixing patch 9 to not depend on patch 7.

Thanks for your patience; I'm not used to making patches. I need to be more careful with these!

[dsizzle]

Consolidated patch for enhancement 2

[dsizzle]

patch for enhancement 9 (character view pane size changing rapidly)

[dsizzle]

I looked into the node monitoring some more. It appears that FontManager is the key, as it is already monitoring for fonts in directories and handling the addition and deletion of them. I don't think it's worth it to duplicate the functionality of FontManager. I could add an accessor function to return FontManager's list of font directories and then monitor those but again I'm not sure if that falls into the scope of this issue. Unless I am missing something, I don't see how BPathFinder would find the font dirs. Thoughts? Alternatively I could add a couple more system font dirs but it wouldn't be as robust as FontManager.

[pulkomandy]

One of the directory constants for BPathFinder is B_FIND_PATH_FONTS_DIRECTORY (defined in FindDirectory.h). The BPathFinder APIs will return a complete list of all font directories in the system, and if we add more, it will return those as well.

For enumerating fonts, the existing APIs can be used (see for example src/preferences/appearance/FontMenu.cpp). As Axel suggested, maybe a lazily built menu would be the simplest solution. To implement this, you would use the "MenusBeginning" hook function (in BWindow) to recreate the whole font menu. This way, there is no need to node monitor anything and you can just get the current list of fonts from the FontManager, like the Appearance preferences do.

[pulkomandy]

2 and 9 applied in hrev48376.

I noticed some problems with the first patch. There are annoying scrolling interactions and off by one errors.

• When you start the application, it scrolls to the end of the character list and shows nothing. But when hovering with the mouse you can see there are some "ghost" characters there (?). It should show the start of the list instead.
• Sometimes the next item in the block list is selected. For example select "Phaistos disk", then hover some characters on it, notice it instead highlights the next item in the block list. Probably happens only with short blocks. All items untill "Hangul Syllabs" are ok, starting from "CJC compatibility ideograms" they are off by one.
• When you hover a character that is not fully visible at the bottom of the list, there is an attempt to scroll it into view automatically. However since this is done on just mouse hover, and it scrolls the list, the mouse may end up above another character. And then the list will scroll again. Sometimes it will jump down by several blocks before it stops. I think the character list should never scroll on mouse hover, only on clicks or mousewheel.

[dsizzle]

Thanks for the feedback! I noticed those too and I will look into them.

[pulkomandy]

Patch 7: update_font_families returns a boolean telling you if something changed. You should check this, and rebuild the menu only if needed (so you need to call update_font_families before clearing the old menu).

[dsizzle]

lazily-build font menu patch, now with check for updated font families

[dsizzle]

Patch to fix off-by-one errors in selection of Unicode block

[dsizzle]

Patch to make app scroll to top of char list, and select first unicode block on app start

[dsizzle]

Found another issue; not sure if I caused it but probably did:

• when first unicode block is selected, selecting any other block doesn't change the character view. Scrolling the character view then allows block selection to function.

[dsizzle]

Patch to fix scrolling issues

[dsizzle]

The last patch was meant to address the last issue I reported but it seems like that might have also been the cause of the hovering scrolling issue.

[pulkomandy]

Applied all fixes in hrev48387. Some comments about commit message formatting:

• The first line should be a short description of the change (70 characters maximum)
• You can have a more detailed explanation below that

I have rewritten your commit messages to better match this format. This makes it easier to browse the changes in the source browser (cgit.haiku-os.org) and other git tools.

So only items 4 and 5 are remaining now.

[dsizzle]

Thanks for the info on the comments and for rewriting mine. I will follow that from now on.

item 4: It doesn't look like BFont::Blocks is implemented even still, which is why I haven't touched it. I could try to tackle that but I'm not sure what exactly it entails.

item 5: axeld says above that it is too much effort for too little gain. Do you agree with that?

[pulkomandy]

BFont::Blocks probably needs us to make use of fontconfig to extract relevant information from the fonts. We need this for other reasons as well (implementing font overlays and picking appropriate fonts for a given language), so if you want to continue your work with bigger tasks, this could be one and would be very appreciated.

For item 5, I think there could be a reasonable solution using a BToolTip (this didn't exist yet when Axel made his comment) or maybe a BPopUpMenu. I think the Microsoft Windows character map has (had?) this and it is a nice feature as it allows you to browse the characters at a small font size, but still get a bigger view when needed.

[dsizzle]

Created issue #11518 (Implement BFont::Blocks) as a related issue. I'll give it a whirl.

[dsizzle]

I can find ToolTip.cpp, but ToolTip.h seems to be missing...did it get removed for some reason? Or accidentally never checked in?

[pulkomandy]

It's not a public class yet, so it is available in headers/private/interface/ToolTip.h, only for applications bundled with Haiku. Other apps are limited to the sime tooltips provided by BView->SetToolTip (with just a string) only, but in this case you need a more "advanced" one to set the font.

[dsizzle]

FYI, #7 has been done for a while (hrev48387)

I'm working on #4 since #11518 is now done.

[dsizzle]

Performance patch; linear lookups converted to binary searches

[dsizzle]

It wasn't part of the original list of enhancements, but there were a couple of places in the code that mentioned using binary searches instead of trolling through the entire kUnicodeBlocks list, so I converted them and added a patch here. It should help performance quite a bit since those lookups are performed quite often as a user navigates the views.

[waddlesplash]

Patch applied in hrev51542.

[pulkomandy]

I have split this in two issues for the remaining items, in order to keep it easier to manage for people looking for tickets in the "easy" list. They usually don't want to read through 11 years of comment to understand an issue.

[nielx]

Assign tickets with status=closed and resolution=fixed within the R1/beta2 development window to the R1/beta2 Milestone