Fixed for popup text wasn't vertically centered

timusus / RecyclerView-FastScroll

A simple FastScroller for Android's RecyclerView

Other

1.39k stars 182 forks source link

Fixed for popup text wasn't vertically centered #101

Closed huikaihoo closed 5 years ago

huikaihoo commented 5 years ago

Fixed for popup text wasn't vertically centered
Fixed integer division issue
Added missing annotations

timusus commented 5 years ago

@huikaihoo thanks..

I believe I resolved this issue earlier today. Can you explain what this commit fixes?

huikaihoo commented 5 years ago

The position of popup text is incorrect for v1.0.21, like below:

screenshot_2019-02-08-22-27-33

The calculation in v1.0.20 is correct in most cases. But for issue #85, the position is wrong because it contains letters (like g, j) which will appear below baseline.

As canvas.drawText take y-coordinate of baseline of the text, and mTextBounds.height() return actual height of text, so it make wrong calculation on vertical position.

More may refer to https://stackoverflow.com/questions/27631736/meaning-of-top-ascent-baseline-descent-bottom-and-leading-in-androids-font

timusus commented 5 years ago

Hmm, I was working on this today, and the end result did not look like your screenshot there. Are you certain that's from 1.0.21? I did test my changes in e8546a547db08aab76d3444b6b4994d3ebd87fbd and it looked OK.

getTextBounds() already accounts for the ascent and descent, if it exists in the text, so I'm not sure why we'd need to use FontMetrics here.

I don't mean to come across as argumentative, it's fine if I'm wrong. But e8546a547db08aab76d3444b6b4994d3ebd87fbd should resolve #85.

huikaihoo commented 5 years ago

Left/Orange is before e8546a5 mBgBounds.height() - (mBgBounds.height() - mTextBounds.height()) / 2 Middle/Green is after e8546a5 mBgBounds.height() / 2 + ((mBgBounds.height() - mTextBounds.height()) / 2 Right/Blue is using FontMetrics (mBgBounds.height() - fontMetrics.ascent - fontMetrics.descent) / 2

Note that issue #85 happen because value pass in to canvas.drawText is y-coordinate of baseline of the text, which is the yellow line in above, the descent part with show below y-coordinate.

My personal opinion is

For text with descent part, right is the best
For text without descent part, left is the best

timusus commented 5 years ago

I see. Thanks for the detailed investigation and explanation.