View Issue Details

IDProjectCategoryView StatusLast Update
0001114OpenMPTUser Interfacepublic2023-06-10 08:40
Reportermanx Assigned To 
PrioritynormalSeverityminorReproducibilityhave not tried
Status newResolutionopen 
Target VersionOpenMPT 1.?? (long term goals) 
Summary0001114: 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:

  • custom paletted bitmap with a cache buffer
  • plain GDI immediate mode
  • plain GDI with a cache buffer
  • GDI+ immediate mode
  • GDI+ with a cache buffer
  • Direct2D immediate mode (although possible, it probably does not make sense to target a cache buffer with Direct2D)

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.

TagsNo 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)
patternfont-ascii-v2.patch (4,369 bytes)   
Has the bug occurred in previous versions?
Tested code revision (in case you know it)

Relationships

related to 0001071 assignedmanx Playback Row cursor disappears on high latency 
related to 0000627 assignedSaga Musix Make OpenMPT completely DPI-aware 
related to 0001003 confirmed Pattern View :: Smoothscrolling enabled :: displays artifacts 
related to 0000522 resolvedmanx Playback Cursor always jitters when you run it under Wine 1.7 
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 

Activities

manx

manx

2018-10-12 10:50

administrator   ~0003660

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.

Saga Musix

Saga Musix

2018-10-12 20:28

administrator   ~0003662

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.
Even if the pattern bitmap would be cached at the beginning of the pattern, drawing the pattern with real fonts can still stall the display update for a noticeable amount of time, as it can be observed when playing fast-paced modules in XMPlay. With bitmap drawing, this issue can be pretty much ruled out.

manx

manx

2018-10-16 09:50

administrator   ~0003672

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.

manx

manx

2018-10-16 09:54

administrator   ~0003673

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)
patternfont-ascii-v1.patch (4,323 bytes)   
harbinger

harbinger

2018-10-22 16:39

reporter   ~0003676

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...

arseniiv

arseniiv

2018-12-28 11:57

reporter   ~0003780

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.)

Screenshot-1704.png (1,647 bytes)   
Screenshot-1704.png (1,647 bytes)   
Saga Musix

Saga Musix

2018-12-28 13:41

administrator   ~0003781

r11123 or later from https://builds.openmpt.org/builds/ should solve that issue.

arseniiv

arseniiv

2018-12-29 10:31

reporter   ~0003783

Thanks! Indeed it does.

ASIKWUSpulse

ASIKWUSpulse

2020-06-03 17:48

reporter   ~0004358

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

Issue History

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