"Highlighting" a section of the screen

[goku420]

I'm working on a pseudo-GUI where my screen is split into panels. Each panel has its own 1MB buffer. Whenever a panel is selected, the drawing functions write to that buffer. Then whenever puts is called, all the buffers are copies to the primary video buffer, which is then copied to video mem. What I'm trying to achieve is whenever a panel is selected, it is drawn in a brighter color than the rest. However I'm having trouble formulating a way of doing this. Here's what I got so far:

inside puts:

Code: Select all

    for (int i = 0; i < 3; ++i)
    {
		auto panelPtr = &panels[i];
		uint32_t depth = vbeinfo->bpp / 8;
		uint8_t* write_ptr = (uint8_t*) video_buffer;
		uint8_t* read_ptr = (uint8_t*) panelPtr->video_buffer;
		for (int y = panelPtr->ybase; y < panelPtr->endingline(); ++y)
		{
			for (int x = panelPtr->xbase; x < panelPtr->endingcolumn(); ++x)
			{
				unsigned where = x * depth + y * vbeinfo->pitch;
				uint8_t color1 = read_ptr[where];
				uint8_t color2 = read_ptr[where + 1];
				uint8_t color3 = read_ptr[where + 2];
				if (panelindex == getpanelindex())
				{
                                        // I suspect these lines might be the issue
					color1 |= 0x555 & 255;
					color2 |= (0x555 >> 8) & 255;
					color3 |= (0x555 >> 16) & 255;
				}
				write_ptr[where] = color1;
				write_ptr[where + 1] = color2;
				write_ptr[where + 2] = color3;
			}
		}
	}
	
    uint8_t* writePtr = (uint8_t*) video_buffer_addr;
    uint8_t* readPtr = (uint8_t*) video_buffer;
    memcpy(writePtr, readPtr, vbeinfo->pitch * rows);	

The idea is to keep each panel's buffer unmodified and only copy the highlighted colors to video_buffer. Being bad with bit-wise manipulation, I think the "color |=" lines are incorrect, but if anybody could spot other issues, it would be appreciated.

Edit: fixed a bug. auto panelPtr = &panels[panelindex]; should be auto panelPtr = &panels;
Edit2: D'oh! That was the stupid bug. panelindex == getpanelindex() is redundant, it should've been i == getpanelindex()

[FallenAvatar]

Obviously, you got these algorithms from somewhere. Where? (link) Why did you choose this algorithm? (text) Etc.

- Monk

[Combuster]

There are quite a few issues with this code. It seems to assume 24 bpp colours in one place while having a generic unused bpp in another. It also assumes that the pitch of all the buffers are the same.

Also, ORing a number with some other number might have no effect at all if those bits were already set, resulting in some skewed colours. OR'ing each of the pixel components with the 0101010101... pattern does tend to result in an overall lighter image, although contrast might get skewed inappropriately.

It might be a better idea to multiply each colour on the non-selected panes by something like 0.75 and darken them, to maintain the full contrast on the foreground. This is also computationally effective to do as the algorithm is simply x - (x >> 2), which would also aid in vectorisation even in 24-bpp colour modes.

[goku420]

Obviously, you got these algorithms from somewhere. Where? (link) Why did you choose this algorithm? (text) Etc.

- Monk

I adapted the codes from MosquitOS although I refactored them into loops.

There are quite a few issues with this code. It seems to assume 24 bpp colours in one place while having a generic unused bpp in another. It also assumes that the pitch of all the buffers are the same.

Also, ORing a number with some other number might have no effect at all if those bits were already set, resulting in some skewed colours. OR'ing each of the pixel components with the 0101010101... pattern does tend to result in an overall lighter image, although contrast might get skewed inappropriately.

It might be a better idea to multiply each colour on the non-selected panes by something like 0.75 and darken them, to maintain the full contrast on the foreground. This is also computationally effective to do as the algorithm is simply x - (x >> 2), which would also aid in vectorisation even in 24-bpp colour modes.

I eventually went with:

Code: Select all

color1 = (color1 & 0xfefefe) >> 1;
color2 = (color2 & 0xfefefe) >> 1;
color3 = (color3 & 0xfefefe) >> 1;

[Combuster]

color1 |= 0x555 & 255;
color2 |= (0x555 >> 8 ) & 255;
color3 |= (0x555 >> 16) & 255;
(...)
color1 = (color1 & 0xfefefe) >> 1;
color2 = (color2 & 0xfefefe) >> 1;
color3 = (color3 & 0xfefefe) >> 1;

It looked like shotgun code and it still looks like shotgun code as it fails sanity checking. Did you happen to notice that the constants used in here are way larger than the variables are allowed to contain? Did you also notice that a 12-bit number shifted right by 16 always yields zero? Such sanity checks can help you spot errors in your thought process, and possibly fix things before they happen next time.