I also noticed that, while in 32 bit mode any resolution is supported, in 16 bit mode horizontal resolution has to be a multiple of 2, and in 8 bit mode it has to be a multiple of 4. Otherwise, the image will be deformed.

The reason is that each line of pixel, in FreeBasic, is supposed to be accessed as a sequence of UNSIGNED LONG. Since this data type is made of 4 bytes, if a line contains a number of bytes that is not a multiple of 4, the remainder will shift the next line, if it is accessed using an ULONG pointer. To prevent this, some bytes are added at the end of each line to keep it aligned.

Unfortunately, in OpenGL, the command glTexSubImage2D (used to copy the buffer to a texture) does not expect those bytes to be here, and so it will consider them as part of the next line, shifting it, and shearing the whole image. It can be prevented by ensuring that each line contains a multiple of four bytes.

In 32 bit mode each pixel uses 4 bytes: so, it does not matter how many pixels are in a line, the number of bytes will always be a multiple of 4. In 16 bit mode, each pixel uses 2 bytes, so as long as the number of pixels per line is even the number of bytes will be a multiple of 4. In 8 bit mode the number of pixel per line just has to be a multiple of 4.

Since OpenGL is a true color mode, the most useful mode is 32 bit; the only reason to use 16 bit or 8 bit is to port old code... that would likely use a legacy screen resolution (and all those resolutions are multiple of 4, horizontally). So, in theory this should not be an issue, but perhaps it might need a warning in documentation

I've written some review comments on the Github pull request for this, but they're only about some minor formatting issues and such that would be nice to have fixed before merging.

Besides that I've looked at whether non-OpenGL graphics modes are affected, but no, it looked good. So we don't have to worry about this patch breaking anything else (which is typically a concern when making changes to FB). Also the new background thread for the new OpenGL mode looks good to me (it's much like other parts of gfxlib2 are implemented already).

While I was applying the suggested changes, I fixed the bug that occurs when the width is not a multiple of 4; looks like the screen buffer has no alignment, and OpenGL expected a multiple of 4. It can be fixed with glPixelStorei(GL_UNPACK_ALIGNMENT, 1), and I added it (I had to add it to the declarations as well)

angros47 wrote:While I was applying the suggested changes, I fixed the bug that occurs when the width is not a multiple of 4; looks like the screen buffer has no alignment, and OpenGL expected a multiple of 4. It can be fixed with glPixelStorei(GL_UNPACK_ALIGNMENT, 1), and I added it (I had to add it to the declarations as well)

angros47 wrote:While I was applying the suggested changes, I fixed the bug that occurs when the width is not a multiple of 4; looks like the screen buffer has no alignment, and OpenGL expected a multiple of 4. It can be fixed with glPixelStorei(GL_UNPACK_ALIGNMENT, 1), and I added it (I had to add it to the declarations as well)

Hi angros47,

I adapted a little demo to test the patch. It works flawlessly here (FB64, IntelHD). You may want to take a look at it:

I made a Pull Request to add the constants to the fbgfx source. (It currently failed though, but I that's because the Travis-CI 32-bit build isn't working) - edit: dkl swooped in and fixed travis.. he must have seen the fb horse signal shining in the night sky ...

I noticed there is no documentation yet.... hint, hint. :)

Looks like there are some rules to using the feature. ScreenControl must be called before Screen/ScreenRes. Looking at the fbgfx source code, looks like some things happen when the driver is initialized, and some checks are made later.

What can be or needs to be set-up before Screen/ScreenRes? Would it make sense to add a flag to Screen/ScreenRes?What can be changed after Screen/ScreenRes? To what effect?

---Off-topic, but there was a question of sorts about the data context variable:- use the data across multiple modules either pass with 1 pointer, or with 1 extern statement- easy for the programmer to identify the global data, i.e. visually when writing the code- windows .dll symbols are private and linux .so are global, so there was some discussion (10 years ago) as to how to either make the library's data private in linux by allocating a context on use, or shared in windows through some memory mapped file, for example.- by having all the shared/global data in a context, the library is an interface for an object, and might be possible create multiple instances; for fbgfx maybe multiple windows