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

FR: [Add more highlight color options] #9024

Open
krillin666 opened this issue Apr 21, 2022 · 30 comments
Open

FR: [Add more highlight color options] #9024

krillin666 opened this issue Apr 21, 2022 · 30 comments

Comments

@krillin666
Copy link

Does your feature request involve difficulty completing a task? Please describe.
I use my Kobo to review a lot of academic papers, as such I rely heavily on highlights to retain the content I've read. However, my workflow involves different types of highlight's colour (yellow, green, red, purple). This is because I use Zotero as my ref manager and use this script to automatically add comments/tasks to my highlights based on the colour of the highlight.

Describe the solution you'd like
It would be fruitful if Koreader added the ability to select highlight colour (even in B/W e-ink reader). With this, we could review the PDFs in other devices without being limited to the rather dull gray highlight, which is the default highlight colour that KOreader adds to pdfs.

Describe alternatives you've considered
I could manually add a comment to each highlight with the colour I wanted it to be and then on PC check all highlights and convert them to the colour mentioned in the note of that highlight. But, that would be obviously tedious and labour.

Additional context
I've mentioned a specific workflow with Zotero, nevertheless I'm sure many people use highlights of different colour in their workflow. So please comment below if you also find this useful !

@kauesena
Copy link

My case is slightly different but would also benefit from different colours for highlights. With different colours, I can distinguish between categories of highlighted passages: excerpts I did not understand, definitions, nice phrases, words I did not know, key quotes, etc.

@mergen3107
Copy link
Contributor

Sounds similar along the lines of this I think: #7756
@kauesena see if some suggestions in the OP might work for you, at least after extraction of notes

@kauesena
Copy link

@mergen3107 thank you for mentioning that issue. How do you add the labels to the highlights? You add them simply as notes? Your use case and mine are essentially the same (though the categories would differ, naturally).

I think another useful thing if colours are difficult to do be implemented, would be creating other highlighting styles such as dashed underline, wobbly underline, box-like highlighting, etc. Perhaps I should submit a separate issue for this.

@mergen3107
Copy link
Contributor

@kauesena
Yes, these are just words I put manually before highlight "categories".

@mergen3107
Copy link
Contributor

@kauesena
Currently, there are four highlight modes in KOReader: highlight, underline, invert, strikeout. Moreover, you can now indicate whether this is not just a highlight, but also a note.

Does this work with your script-based workflow now?

@poire-z
Copy link
Contributor

poire-z commented May 10, 2023

Our UI code doesn't support colors, everything uses gray. I once wished we could have colors for the calendar view, #5854 (comment) - but it would be a pain to update all the blitting code to support colors for the very few places we may actually use it (calendar, highlights), and the very few color devices as most ones are Black and White.
Only our engines (mupdf, crengine) page drawing code, which is C code in libraries, can do it - so we get it for book content on color Pocketbooks and Android.

@yosoyyognu
Copy link

yosoyyognu commented Sep 24, 2023

I use boox ultra C and i bought it because of colors. I would be very happy if you add colors to the highlights!

By the way, this is the best reader...

Regards

@poire-z
Copy link
Contributor

poire-z commented Sep 30, 2023

We can't have colors in the UI without some really extensive rework :/
We got to have colors in document pages more easily.
But highlights are drawn over the document pages by the UI code.

@Galunid
Copy link
Member

Galunid commented Oct 2, 2023

I played around with this for a bit and had some initial success.

image

Simple idea was to create paintColorRect in ffi/blitbuffer.lua and BB_fill_rect_color in blitbuffer.c. We could have paintRect that calls paintGrayscaleRect (current implementation) and paintColorRect (which would be new implementation). @poire-z Would we be fine with this?

@poire-z
Copy link
Contributor

poire-z commented Oct 2, 2023

@poire-z Would we be fine with this?

I'm not the authority on this :) @NiLuJe is.

(The risk is that if we start doing color, we'll want colored text, color hatching... and everything will be duplicated - and in frontend, we'll have to have code paths for color devices, to disable for gray devices - ie. the list of type of highlight will be 4 items for gray devices, but could get 32 items for colors, and the need to have a scrollable container, etc... - and most people, devs & users, won't test and care.)

@NiLuJe
Copy link
Member

NiLuJe commented Oct 3, 2023

I'd have to see it to judge, but, yeaaaah, I'm also not necessarily enthused about it ;p.

@smasher816
Copy link

@Galunid would you mind sharing the diff?

@Galunid
Copy link
Member

Galunid commented Oct 25, 2023

@smasher816 Sure
base.txt
frontend.txt

Change extension from .txt to .diff. Github wouldn't let me upload them otherwise. It's just a hacked solution that may break on other devices (it works on emulator)

@smasher816
Copy link

Thanks Galunid. I took your base and tried polishing it up a bit.

Here is a video showing it off in action on an Onyx Tab Mini C.
https://github.com/koreader/koreader/assets/904012/531f5e86-27a8-4f56-94c3-58fe24054887

Here is a MR with my changes.
https://github.com/koreader/koreader/pull/11044/files
https://github.com/koreader/koreader-base/pull/1680/files

Current implementation now uses a is_rgb flag attached to each color type. This lets us keep a single paintRect lua function, that calls into the correct underlying Cbb function depending on if we are passing in an rgb color or not.
For devices without 32bit support or using the legacy lua blitting, no changes occur.

The frontend highlight code will respect the "Use color" setting from the Screen settings menu. If disabled a B&W color is passed in (as before) and if enabled then a ColorRGB32 is passed in causing the new paths to be called.

Additionally, a "Highlight color" option (with translations) was added next to the current opacity option in the settings menu. This provides a few preset values, similar to the kindle app or the native drawing app.

I think the above function shows off the features of koreader on a color e-ink tablet without adding too much feature creep, and without duplicating many functions/code.

@smasher816
Copy link

Here is a quick photo for those that don't want a video.
PXL_20231026_170917059

I haven't bothered trying to do "different colors per highlight" since it's not personally something I use.
If the above MR's laying the foundation get through, then maybe someone else could work on it in the future.

@smasher816
Copy link

Individual colors actually wasn't too difficult to accomplish. Thanks @hius07 for the pointer on how "per highlight" styles were saved.
I'll continue to work on getting the MR pushed through.

PXL_20231028_041158819

@uroybd
Copy link
Contributor

uroybd commented Dec 19, 2023

This is nice. More context to organize one's highlights.

@Bluemoondragon07
Copy link

How can I generate the apk with these modifications to test on Android?

@Galunid
Copy link
Member

Galunid commented Jan 14, 2024

@Bluemoondragon07

  1. Clone the repo
  2. Pull the changes from relevant PRs (Add color highlight menu #11044, Add a few RGB-aware set & blend methods to the blitters koreader-base#1680), see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally
  3. Follow instructions under https://github.com/koreader/koreader#development

If you are just interested in the app, here it is https://gofile.io/d/u5C5je. It's signed with my key, so you'll have to uninstall KOReader first. It's build directly from the PRs linked, so it won't have changes after November 2023.

@Bluemoondragon07
Copy link

@Galunid Thank you! Unfortunately, the app linked could not install on my Android 11 Meebook, Android 11 Bigme, or Android 12 Samsung. It crashed when I tried it with on Windows with Bluestacks.

But, I cloned the KOreader repo. Not sure what to do about the KOreader-base modifications, but I'm getting there. Thanks for the direction!

@Galunid
Copy link
Member

Galunid commented Jan 18, 2024

@Bluemoondragon07 Sorry about that. I messed up signing the apk and it doesn't work with android 11+. This one works (at least on my Android 12 device). https://gofile.io/d/GTfm79
Let me know if you need other architectures than the arm.

@Bluemoondragon07
Copy link

Thank you so much, @Galunid .

I was able to install on the Bigme Inknote and test things out! I feel so privileged.

Some minor drawbacks like the color making the note underline invisible, highlight transparency being made void, and other things, but wow--this is a massively helpful implementation. Thanks!

Now I'm gonna try to generate an up-to-date apk with the functions.

@NiLuJe
Copy link
Member

NiLuJe commented Jan 18, 2024

Now I'm gonna try to generate an up-to-date apk with the functions.

Don't, this is next on my TODO list after koreader/koreader-base#1729

Your time will be better spent actually testing how it looks like & feels, as I have zero interest in this myself besides merging the sets of PR ;).

@Bluemoondragon07
Copy link

Ah, okay. Couldn't get build_release.sh to work, anyways. On the other hand, I can definitely give feedback on your current version 👍🏽

@uroybd
Copy link
Contributor

uroybd commented Mar 10, 2024

Any update on this?

@poire-z
Copy link
Contributor

poire-z commented Mar 10, 2024

I think @NiLuJe is a bit busy, but may welcome some testing on his updates to the related koreader-base PR koreader/koreader-base#1680.

@NiLuJe
Copy link
Member

NiLuJe commented Mar 10, 2024

Yup, it was basically ready last I checked, I was just letting it sit before review (I may also have been waiting out a feature freeze at one point, can't recall).

But, sure, if you can test it on an actual device, go ahead, as I don't have the hardware ;).

@kauesena
Copy link

Is there an apk? I'd happily test it on my colour e-ink device (Boox Note Air 3 C).

@uroybd
Copy link
Contributor

uroybd commented Mar 10, 2024 via email

@Aparikk
Copy link

Aparikk commented Mar 21, 2024

I'd like to test it on my pocketbook inkpad color 3. Is there any way to do so atm?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests