Support non-monospace/monospace font rendering - #207
Support non-monospace/monospace font rendering#207beichen (BeichenDream) wants to merge 8 commits into
Conversation
|
Now it supports the rendering of non-monospaced fonts
|
|
beichen (@BeichenDream) Seems most changes in TerminalPanel.java are caused by different formatting options (4 spaces instead of 2, etc.) Could you please adjust your formatting options to keep meaningful changes only? |
Do you mean code style? |
|
Yes, code style |
fix code style
Now that the code has been fixed, you can test it |
|
beichen (@BeichenDream) Unfortunately, the diff is still large (3216 lines). Can you please checkout the original TerminalPanel, apply your changes and push it without reformatting the whole file? |
fix code style
|
Alright, I have manually fixed the code style now. Now you can test the code. |
|
|
||
| CharBuffer buf = new CharBuffer(c); | ||
| int xCoord = x * myCharSize.width + getInsetX(); | ||
| int xCoord = computexCoord(x, y) + getInsetX(); |
There was a problem hiding this comment.
Unfortunately, it will cost some performance, because now painting (height,width) panel would require approximately (height * width * width) / 2 calls of gfx.getFontMetrics(font).charWidth(<char>).
There was a problem hiding this comment.
Yes, this is a must for non-monospaced fonts;
There is another point that can be cached after rendering. If Terminal does not receive keyboard messages or mouse messages or the size of the window changes, you can directly return the results of the last rendering. This can solve the performance problem.
For me, the rendering of non-monospaced fonts is necessary because it is impossible for our program to carry font files of dozens of megabytes.
There was a problem hiding this comment.
You can build jediterm.jar from your fork and use it as a workaround. When proper support for non-monospaced is added, you can remove your fork. Will it work for you?
There was a problem hiding this comment.
You can build jediterm.jar from your fork and use it as a workaround. When proper support for non-monospaced is added, you can remove your fork. Will it work for you?
I have seen a large number of non-monospaced fonts rendering issues in issues. If you don’t solve it, I think you should understand.
There was a problem hiding this comment.
Sorry, not sure I understand: if your software would use jediterm.jar built from your fork, it would solve non-monospaced fonts rendering issues for your users. Right?
There was a problem hiding this comment.
Regarding this change, fixing non-monospaced fonts rendering issues as it was suggested in this PR would lead to degraded performance for all fonts. I agree that non-monospaced fonts rendering issues should be fixed, but probably the fix should be more elaborate to not degrade performance for monospaced fonts. No particular plans, but I'll think about it.
There was a problem hiding this comment.
Regarding this change, fixing non-monospaced fonts rendering issues as it was suggested in this PR would lead to degraded performance for all fonts. I agree that non-monospaced fonts rendering issues should be fixed, but probably the fix should be more elaborate to not degrade performance for monospaced fonts. No particular plans, but I'll think about it.
Please do not close this PR. I should have time to fix performance issues in the next few months. Add double buffering and rendering cache to it.
There was a problem hiding this comment.
Sure, let's keep it open. Not sure double buffering would help here. It seems to me that it's more about improving algorithm of calculating x-position of a cell.
There was a problem hiding this comment.
Optimal calculation of X coordinate algorithm
Now it only accounts for 3.6% of the CPU in the entire drawing cycle
There was a problem hiding this comment.
The algorithm has been optimized and now only takes up very little CPU
|
Thanks, I see your idea. I think fixing it for non-monospaced fonts shouldn't degrade performance for monospaced fonts. |
Yes, this is a must for non-monospaced fonts; There is another point that can be cached after rendering. If Terminal does not receive keyboard messages or mouse messages or the size of the window changes, you can directly return the results of the last rendering. This can solve the performance problem. For me, the rendering of non-monospaced fonts is necessary because it is impossible for our program to carry font files of dozens of megabytes. You can also create a method in the object TerminalPanel that indicates whether to perform non-monospace font rendering |
Optimal calculation of X coordinate algorithm Now it only accounts for 3.6% of the CPU in the entire drawing cycle
|
I think you can merge branches now |
|
I've reviewed the lastest commit (99db2b4) and there are still |
beichen (BeichenDream)
left a comment
beichen (BeichenDream)
left a comment
There was a problem hiding this comment.
I've reviewed the lastest commit (99db2b4) and there are still
(height * width * width) / 2calls ofgfx.getFontMetrics(font).charWidth(<char>).
I've reviewed the lastest commit (99db2b4) and there are still
(height * width * width) / 2calls ofgfx.getFontMetrics(font).charWidth(<char>).
Yes, this is indispensable at the moment, but it only occupies 3% of the cpu of the drawing cycle. I think it’s good for not being able to render non-monospaced fonts.
|
|
||
| CharBuffer buf = new CharBuffer(c); | ||
| int xCoord = x * myCharSize.width + getInsetX(); | ||
| int xCoord = computexCoord(x, y) + getInsetX(); |
There was a problem hiding this comment.
Optimal calculation of X coordinate algorithm
Now it only accounts for 3.6% of the CPU in the entire drawing cycle
|
|
||
| CharBuffer buf = new CharBuffer(c); | ||
| int xCoord = x * myCharSize.width + getInsetX(); | ||
| int xCoord = computexCoord(x, y) + getInsetX(); |
There was a problem hiding this comment.
The algorithm has been optimized and now only takes up very little CPU
|
3.6% is not much indeed, however, it'd be great to have 0% for monospaced fonts (if that's possible). |
I think we should not spend time on this 3.6% trivial matter. I use jvisualvm to analyze the program and found that most of the time and the cpu are executing the sun.java2d.SunGraphics2D.drawChars() method, so we currently optimize the program the most A good way is to increase the cache to reduce painting |
|
In fact, we have now removed the calculation of wide characters, so it is faster than the original program. |
|
Fix the pull-up progress bar cannot be rendered, reduce useless code calculations |
2 participants
Fix bug
issue