View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0001114 | OpenMPT | User Interface | public | 2018-04-15 17:24 | 2024-08-16 13:30 |
Reporter | manx | Assigned To | |||
Priority | normal | Severity | minor | Reproducibility | have not tried |
Status | new | Resolution | open | ||
Target Version | OpenMPT 1.?? (long term goals) | ||||
Summary | 0001114: Rewrite pattern view drawing | ||||
Description | Pattern view is currently drawn using our own paletted bitmap drawing functionality, ultimately blitting to GDI without any caching of the drawn pattern. Scrolling is handled via ScrollWindow (or redrawing the whole the hole thing for platforms or settings where this will cause problems, like Wine). This whole thing probably needs a more or less complete rewrite or refactoring. There are multiple ways to rewrite it:
GDI+ can be used safely and unconditionally. It is supported on all platforms we support. Direct2D has the advantage of being GPU-accelerated, especially on Windows 8 or later. We still need to provide a fallback for situations without Direct2D acceleration (where it would be slow) though. All things considered, it is probably the best idea to abstract the actual rendering backend. We currently already kind of have such an abstraction in place in the form of CFastBitmap. It probably needs a lot of changes to its interface to be really usable as a true abstraction layer though. | ||||
Tags | No tags attached. | ||||
Attached Files | patternfont-ascii-v2.patch (4,369 bytes)
Index: mptrack/Draw_pat.cpp =================================================================== --- mptrack/Draw_pat.cpp (revision 10909) +++ mptrack/Draw_pat.cpp (working copy) @@ -272,6 +272,16 @@ void CViewPattern::DrawLetter(int x, int y, char letter, int sizex, int ofsx) { const PATTERNFONT *pfnt = PatternFont::currentFont; + + if(pfnt->dibASCII) + { + if(32 <= letter && letter <= 127) + { + m_Dib.TextBlt(x, y, sizex, pfnt->spacingY, (((unsigned char)letter) * pfnt->nOctaveWidth) + ofsx, 0, pfnt->dibASCII); + return; + } + } + int srcx = pfnt->nSpaceX, srcy = pfnt->nSpaceY; if ((letter >= '0') && (letter <= '9')) Index: mptrack/PatternFont.cpp =================================================================== --- mptrack/PatternFont.cpp (revision 10909) +++ mptrack/PatternFont.cpp (working copy) @@ -24,6 +24,7 @@ const PATTERNFONT gDefaultPatternFont = { nullptr, + nullptr, 92,13, // Column Width & Height 0,0, // Clear location 130,8, // Space Location. @@ -53,6 +54,7 @@ const PATTERNFONT gSmallPatternFont = { nullptr, + nullptr, 70,11, // Column Width & Height 92,0, // Clear location 130,8, // Space Location. @@ -82,6 +84,7 @@ const PATTERNFONT *PatternFont::currentFont = nullptr; static MODPLUGDIB customFontBitmap; +static MODPLUGDIB customFontBitmapASCII; static void DrawChar(HDC hDC, WCHAR ch, int x, int y, int w, int h) { @@ -135,6 +138,7 @@ previousFont = font; DeleteFontData(); pf.dib = &customFontBitmap; + pf.dibASCII = nullptr; // Upscale built-in font? if(builtinFont != nullptr) @@ -293,6 +297,8 @@ pf.paramLoMargin = 0; // Margin for second digit of parameter pf.spacingY = charHeight; + { + pf.dib->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); pf.dib->bmiHeader.biWidth = ((width + 7) & ~7); // 4-byte alignment pf.dib->bmiHeader.biHeight = -(int32_t)height; @@ -397,9 +403,58 @@ } hDC.SelectObject(oldBitmap); + DeleteBitmap(bitmap); + + } + + { + + pf.dibASCII = &customFontBitmapASCII; + + pf.dibASCII->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); + pf.dibASCII->bmiHeader.biWidth = ((charWidth * 128 + 7) & ~7); // 4-byte alignment + pf.dibASCII->bmiHeader.biHeight = -(int32_t)charHeight; + pf.dibASCII->bmiHeader.biSizeImage = pf.dibASCII->bmiHeader.biWidth * charHeight / 2; + pf.dibASCII->bmiHeader.biPlanes = 1; + pf.dibASCII->bmiHeader.biBitCount = 4; + pf.dibASCII->bmiHeader.biCompression = BI_RGB; + pf.dibASCII->lpDibBits = new uint8_t[pf.dibASCII->bmiHeader.biSizeImage]; + pf.dibASCII->bmiColors[0] = rgb2quad(RGB(0x00, 0x00, 0x00)); + pf.dibASCII->bmiColors[15] = rgb2quad(RGB(0xFF, 0xFF, 0xFF)); + + uint8_t *data = nullptr; + HBITMAP bitmap = ::CreateDIBSection(hDC, (BITMAPINFO *)&pf.dibASCII->bmiHeader, DIB_RGB_COLORS, (void **)&data, nullptr, 0); + if(!bitmap) + { + hDC.SelectObject(oldFont); + gdiFont.DeleteObject(); + hDC.DeleteDC(); + currentFont = &gDefaultPatternFont; + return; + } + HGDIOBJ oldBitmap = hDC.SelectObject(bitmap); + + hDC.FillSolidRect(0, 0, pf.dibASCII->bmiHeader.biWidth, pf.dibASCII->bmiHeader.biHeight, RGB(0xFF, 0xFF, 0xFF)); + hDC.SetTextColor(RGB(0x00, 0x00, 0x00)); + hDC.SetBkColor(RGB(0xFF, 0xFF, 0xFF)); + hDC.SetTextAlign(TA_TOP | TA_LEFT); + + for(uint32 c = 32; c < 128; ++c) + { + DrawChar(hDC, (char)(unsigned char)c, charWidth * c, 0, charWidth, charHeight); + } + ::GdiFlush(); + + std::memcpy(pf.dibASCII->lpDibBits, data, pf.dibASCII->bmiHeader.biSizeImage); + + hDC.SelectObject(oldBitmap); + DeleteBitmap(bitmap); + + } + hDC.SelectObject(oldFont); gdiFont.DeleteObject(); - DeleteBitmap(bitmap); + hDC.DeleteDC(); } @@ -408,6 +463,8 @@ { delete[] customFontBitmap.lpDibBits; customFontBitmap.lpDibBits = nullptr; + delete[] customFontBitmapASCII.lpDibBits; + customFontBitmapASCII.lpDibBits = nullptr; } OPENMPT_NAMESPACE_END Index: mptrack/PatternFont.h =================================================================== --- mptrack/PatternFont.h (revision 10909) +++ mptrack/PatternFont.h (working copy) @@ -19,6 +19,7 @@ struct PATTERNFONT { MODPLUGDIB *dib; + MODPLUGDIB *dibASCII; // optional int nWidth, nHeight; // Column Width & Height, including 4-pixels border int nClrX, nClrY; // Clear (empty note) location int nSpaceX, nSpaceY; // White location (must be big enough) | ||||
Has the bug occurred in previous versions? | |||||
Tested code revision (in case you know it) | |||||
related to | 0001071 | assigned | manx | Playback Row cursor disappears on high latency |
related to | 0000627 | assigned | Saga Musix | Make OpenMPT completely DPI-aware |
related to | 0001003 | confirmed | Pattern View :: Smoothscrolling enabled :: displays artifacts | |
related to | 0000522 | resolved | manx | Playback Cursor always jitters when you run it under Wine 1.7 |
related to | 0001586 | new | Skinning and/or Dark Mode | |
related to | 0001024 | new | Relics When Navigating to Next Pattern | |
related to | 0001351 | new | printing option | |
related to | 0001390 | new | Pattern zoom | |
related to | 0001508 | new | Highlight selected instrument in the pattern | |
related to | 0001523 | new | Option to hide/show different columns in the pattern. | |
related to | 0001623 | new | [Feature request] FontDot for built-in fonts | |
related to | 0000783 | new | cross-platform OpenMPT | |
related to | 0001710 | new | Adjustable note column width in the pattern editor to enable convenient microtonal notation | |
related to | 0001769 | new | (suggestion) Reduce the space used by the pattern detail toggles by combining all three options into one button |
Another issue that would easily be fixed when using a proper font drawing backend would be the issue of missing character support for custom tuning note names. |
|
For the sake of speed I would very much prefer to keep fonts as bitmaps, but of course they could be upgraded to proper truecolor bitmaps (for anti-aliased fonts) at some point. |
|
Using caching bitmaps implies using a complete ligature-aware and subpixel-aware font cache, which is a huge investment to develop. I would very much like to see a rewrite use modern font rendering APIs (which basically implies using Direct2D/DirectWrite). XMPlay (XMPlay itself, as well as xmp-openmpt) just uses GDI. I do not think we can draw performance conclusions from that. It might very well turn out that, even if done properly and on modern hardware, that Direct2D/DirectWrite is too slow. I just do not think we should rule out the most modern way to render things without testing the performance first. Using a somewhat outdated APi to develop a new or improved feature will not really move things forward. In any case, in a first iteration, we can keep the old pattern drawing path completely as is, as an alternative/fallback solution. |
|
This patch at least adds full US-ASCII support for rendering custom note names with a system font to the current old implementation, which I think we should include as soon as possible. Otherwise, custom note names continue to be rather difficult to even use at all. patternfont-ascii-v1.patch (4,323 bytes)
Index: mptrack/Draw_pat.cpp =================================================================== --- mptrack/Draw_pat.cpp (revision 10900) +++ mptrack/Draw_pat.cpp (working copy) @@ -272,6 +272,16 @@ void CViewPattern::DrawLetter(int x, int y, char letter, int sizex, int ofsx) { const PATTERNFONT *pfnt = PatternFont::currentFont; + + if(pfnt->dibASCII) + { + if(32 <= letter && letter <= 127) + { + m_Dib.TextBlt(x, y, sizex, pfnt->spacingY, (((unsigned char)letter) * pfnt->nOctaveWidth) + ofsx, 0, pfnt->dibASCII); + return; + } + } + int srcx = pfnt->nSpaceX, srcy = pfnt->nSpaceY; if ((letter >= '0') && (letter <= '9')) Index: mptrack/PatternFont.cpp =================================================================== --- mptrack/PatternFont.cpp (revision 10900) +++ mptrack/PatternFont.cpp (working copy) @@ -24,6 +24,7 @@ const PATTERNFONT gDefaultPatternFont = { nullptr, + nullptr, 92,13, // Column Width & Height 0,0, // Clear location 130,8, // Space Location. @@ -53,6 +54,7 @@ const PATTERNFONT gSmallPatternFont = { nullptr, + nullptr, 70,11, // Column Width & Height 92,0, // Clear location 130,8, // Space Location. @@ -82,6 +84,7 @@ const PATTERNFONT *PatternFont::currentFont = nullptr; static MODPLUGDIB customFontBitmap; +static MODPLUGDIB customFontBitmapASCII; static void DrawChar(HDC hDC, WCHAR ch, int x, int y, int w, int h) { @@ -135,6 +138,7 @@ previousFont = font; DeleteFontData(); pf.dib = &customFontBitmap; + pf.dibASCII = nullptr; // Upscale built-in font? if(builtinFont != nullptr) @@ -293,6 +297,8 @@ pf.paramLoMargin = 0; // Margin for second digit of parameter pf.spacingY = charHeight; + { + pf.dib->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); pf.dib->bmiHeader.biWidth = ((width + 7) & ~7); // 4-byte alignment pf.dib->bmiHeader.biHeight = -(int32_t)height; @@ -397,9 +403,58 @@ } hDC.SelectObject(oldBitmap); + DeleteBitmap(bitmap); + + } + + { + + pf.dibASCII = &customFontBitmapASCII; + + pf.dibASCII->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); + pf.dibASCII->bmiHeader.biWidth = ((charWidth * 128 + 7) & ~7); // 4-byte alignment + pf.dibASCII->bmiHeader.biHeight = -(int32_t)charHeight; + pf.dibASCII->bmiHeader.biSizeImage = pf.dibASCII->bmiHeader.biWidth * charHeight / 2; + pf.dibASCII->bmiHeader.biPlanes = 1; + pf.dibASCII->bmiHeader.biBitCount = 4; + pf.dibASCII->bmiHeader.biCompression = BI_RGB; + pf.dibASCII->lpDibBits = new uint8_t[pf.dibASCII->bmiHeader.biSizeImage]; + pf.dibASCII->bmiColors[0] = rgb2quad(RGB(0x00, 0x00, 0x00)); + pf.dibASCII->bmiColors[15] = rgb2quad(RGB(0xFF, 0xFF, 0xFF)); + + uint8_t *data = nullptr; + HBITMAP bitmap = ::CreateDIBSection(hDC, (BITMAPINFO *)&pf.dibASCII->bmiHeader, DIB_RGB_COLORS, (void **)&data, nullptr, 0); + if(!bitmap) + { + hDC.SelectObject(oldFont); + gdiFont.DeleteObject(); + hDC.DeleteDC(); + currentFont = &gDefaultPatternFont; + return; + } + HGDIOBJ oldBitmap = hDC.SelectObject(bitmap); + + hDC.FillSolidRect(0, 0, width - 4, height, RGB(0xFF, 0xFF, 0xFF)); + hDC.SetTextColor(RGB(0x00, 0x00, 0x00)); + hDC.SetBkColor(RGB(0xFF, 0xFF, 0xFF)); + hDC.SetTextAlign(TA_TOP | TA_LEFT); + + for(uint32 c = 32; c < 128; ++c) + { + DrawChar(hDC, (char)(unsigned char)c, charWidth * c, 0, charWidth, charHeight); + } + ::GdiFlush(); + + std::memcpy(pf.dibASCII->lpDibBits, data, pf.dibASCII->bmiHeader.biSizeImage); + + hDC.SelectObject(oldBitmap); + DeleteBitmap(bitmap); + + } + hDC.SelectObject(oldFont); gdiFont.DeleteObject(); - DeleteBitmap(bitmap); + hDC.DeleteDC(); } @@ -408,6 +463,8 @@ { delete[] customFontBitmap.lpDibBits; customFontBitmap.lpDibBits = nullptr; + delete[] customFontBitmapASCII.lpDibBits; + customFontBitmapASCII.lpDibBits = nullptr; } OPENMPT_NAMESPACE_END Index: mptrack/PatternFont.h =================================================================== --- mptrack/PatternFont.h (revision 10900) +++ mptrack/PatternFont.h (working copy) @@ -19,6 +19,7 @@ struct PATTERNFONT { MODPLUGDIB *dib; + MODPLUGDIB *dibASCII; // optional int nWidth, nHeight; // Column Width & Height, including 4-pixels border int nClrX, nClrY; // Clear (empty note) location int nSpaceX, nSpaceY; // White location (must be big enough) |
|
Just from a user's point-of-view, don't forget that there are two, well, THREE priorities from our side (strictly end users). First, whatever it takes to preserve CPU speed. I've been tolerating OpenMPT's text-based interface since the beginning because at least it was showing me the data quickly and in sync with the music as it played. Because i use plugins a lot, i am often running into playback choppiness because of CPU overload. Let's not make it worse... Second priority is customizability. Again i've tolerated text-based music production because at least i have some options as to the appearance of the data in the patterns. Because my music is based on the efficacy of getting my inspirations into the computer and developing them before they go away, how i want the interface to react to what i want can make or break a session. Third, given the satisfaction of the first two priorities, i will support — and promote — any innovation which makes it easier for future expandibility. For example, we may not have a Piano Roll interface in the PE, but if new code can be introduced to make this feature request simply EASIER to implement, i'm all for it. Perhaps we can talk about the concept of add-ons for OpenMPT. Just my 2 cents for your upcoming ideas and plans... |
|
Hello. First of all, thank you for including this addition in 1.28! But I run into some rendering bug when using “&” in note names. Here in the screenshot it’s rendered as an inverted-color pipe. (Also in the tuning editor it’s predictably turned into an underline.) |
|
r11123 or later from https://builds.openmpt.org/builds/ should solve that issue. |
|
Thanks! Indeed it does. |
|
This issue reminded me of a possible related thing: I've noticed in some of my old screen recordings and discords screen-sharing, those times sammie streamed SSS vote-listenings through the voice-chat, would make the graphics have a hard time keeping up with the pattern scrolling. It makes the pattern look like liquid or loose springy fragments :D |
|
Date Modified | Username | Field | Change |
---|---|---|---|
2018-04-15 17:24 | manx | New Issue | |
2018-04-15 17:24 | manx | Relationship added | related to 0001071 |
2018-04-15 17:25 | manx | Relationship added | related to 0000627 |
2018-04-15 17:25 | manx | Relationship added | related to 0001003 |
2018-04-15 17:25 | manx | Relationship added | related to 0000522 |
2018-04-18 14:43 | Saga Musix | Relationship added | related to 0001024 |
2018-10-12 10:50 | manx | Note Added: 0003660 | |
2018-10-12 20:28 | Saga Musix | Note Added: 0003662 | |
2018-10-16 09:50 | manx | Note Added: 0003672 | |
2018-10-16 09:54 | manx | File Added: patternfont-ascii-v1.patch | |
2018-10-16 09:54 | manx | Note Added: 0003673 | |
2018-10-16 12:31 | manx | File Added: patternfont-ascii-v2.patch | |
2018-10-22 16:39 | harbinger | Note Added: 0003676 | |
2018-12-28 11:57 | arseniiv | File Added: Screenshot-1704.png | |
2018-12-28 11:57 | arseniiv | Note Added: 0003780 | |
2018-12-28 13:41 | Saga Musix | Note Added: 0003781 | |
2018-12-29 10:31 | arseniiv | Note Added: 0003783 | |
2020-06-03 17:48 | ASIKWUSpulse | Note Added: 0004358 | |
2020-08-14 13:26 | manx | Relationship added | related to 0001351 |
2020-11-20 17:28 | manx | Relationship added | related to 0001390 |
2021-09-26 16:01 | manx | Relationship added | related to 0001508 |
2021-12-05 20:14 | manx | Relationship added | related to 0001523 |
2022-08-14 14:14 | manx | Relationship added | related to 0001623 |
2022-11-06 06:40 | manx | Relationship added | related to 0000783 |
2023-06-10 08:40 | manx | Relationship added | related to 0001710 |
2024-04-01 15:54 | manx | Relationship added | related to 0001769 |
2024-08-16 13:30 | manx | Relationship added | related to 0001586 |