domnulvlad / TLBFISLib

Arduino library for displaying custom information on the monochrome display (FIS/DIS) located in the instrument cluster of VAG vehicles, via 3LB / TLB / TWB
GNU General Public License v3.0
5 stars 0 forks source link

why not XOR mode? #1

Open tomaskovacik opened 4 months ago

tomaskovacik commented 4 months ago

I am using XOR mode for sending bitmaps in vag_door_indicator project for opening/closing doors, with my vagfiswriter lib, in your lib I can make it work if I set:

FIS.setDrawColor(FIS.INVERTED); FIS.setBitmapTransparency(FIS.TRANSPARENT); before sending each door bitmap

if I send it only with FIS.setDrawColor(FIS.INVERTED); when what is 1 is set to be black on screen, which lets say make sens, not that inverted should flit 0=>1 and 1=> 0, but ok then If I send it only with FIS.setBitmapTransparency(FIS.TRANSPARENT), that make sens named as transparent

naming this mode XOR from the original documentation makes more sense I cannot image any other easy way how to explay that cluste ris something inverting and then merging or merging and then inverting :)

anyway you can test it out: https://github.com/tomaskovacik/vag_door_indicator/tree/master/SW/2.0sw/var_door_indicator, A0 -> A5 are door switches, activated when pulled to ground

domnulvlad commented 4 months ago

Well that's what I named them when I first started this project. To me, natural language makes more sense than bitwise operations.

And you don't need to do things like setDrawColor, setFont, setTextAlignment etc. before every write, you can set them once and they persist.

INVERTED refers to the draw color of text or bitmaps, not to a "mode" like XOR or something. It means that writing something (text/bitmap) will cause red pixels to be black and black pixels to be red on the FIS. Originally the draw colors were RED and BLACK, to indicate the text color, but it didn't make sense for bitmaps.

Then TRANSPARENT for bitmaps means that, if you draw the bitmap over something else, where there are black pixels in the image you will see the content from the background coming through.

Now the fact that combining these two flags creates "XOR mode" sort of makes sense for me... If you really want, I could add a function like setBitmapMode(XOR) that just sets those two flags, but I consider that more confusing.

domnulvlad commented 4 months ago

Noticed something in your code, after you draw the logo, you delay with the normal function. You should make a non-blocking delay with millis(), and call FIS.update() every loop in there. And there is no point in doing FIS.clear() before FIS.turnOff(), you might just get a blank screen for a few milliseconds.

And why did you comment out if (Radio.hasData())? This will make it so radio data is written to the FIS every loop, even if it's old data. The "request" code will never trigger, if the radio decides to stop sending data for some reason. And if somehow, it doesn't send the "off" message when you turn it off (happens on my radio, seen with logic analyzer), the radio data will remain in the FIS because the "request" code with the 5 attempts doesn't happen.

tomaskovacik commented 4 months ago

Well that's what I named them when I first started this project. To me, natural language makes more sense than bitwise operations.

And you don't need to do things like setDrawColor, setFont, setTextAlignment etc. before every write, you can set them once and they persist.

INVERTED refers to the draw color of text or bitmaps, not to a "mode" like XOR or something. It means that writing something (text/bitmap) will cause red pixels to be black and black pixels to be red on the FIS. Originally the draw colors were RED and BLACK, to indicate the text color, but it didn't make sense for bitmaps.

Then TRANSPARENT for bitmaps means that, if you draw the bitmap over something else, where there are black pixels in the image you will see the content from the background coming through.

Now the fact that combining these two flags creates "XOR mode" sort of makes sense for me... If you really want, I could add a function like setBitmapMode(XOR) that just sets those two flags, but I consider that more confusing.

it is your project, so :)

I take it from here, the info is from a document I happened to have in my Google Drive :) https://github.com/tomaskovacik/VAGFISWriter/blob/83322335c3d227fa9fb5bb15b735150a4f97fd23/src/VAGFISWriter.cpp#L384

I am not questioning that combining some data handling should/will make XOR, I am questioning the decision to hide it behind two modes which make no one think about it as simple XOR without proper explanation(how are data handled for real, what comes first and so on), I was forced to look in library source to find out how to make it work

but again we are just guessing here ..... so what do we know :D

I will try some test to try understand it

domnulvlad commented 4 months ago

It wasn't really a "decision". XOR mode is a side effect of two bits being changed in the mode byte, which would overwrite the INVERTED and TRANSPARENT options.

The real problem is, I can't find time to write proper documentation. That is what I wanted to do in the Wiki section of the Github page. So that's true, I left it without explanation.

Maybe I should separate setDrawColor into setTextColor and setBitmapColor, so then I could add setBitmapMode(XOR) without affecting the text color. And even leave setDrawColor and make it call those two functions to keep the original behavior for old code.

tomaskovacik commented 4 months ago

Noticed something in your code, after you draw the logo, you delay with the normal function. You should make a non-blocking delay with millis(), and call FIS.update() every loop in there. And there is no point in doing FIS.clear() before FIS.turnOff(), you might just get a blank screen for a few milliseconds. yes, that is old code which is actually never used (logo =0), I am shipping out modules based on atmega88 and with that huge bitmap it just did not fit (12k flash required), probably can be fixed somehow.... but why :) also 3secunds is a safe margin, the cluster will auto reset it self after 4 seconds or maybe it is 3? o_O

regarding clear/off I was testing and without it I got artefacts on 1st line of tripcomuputer mode half screen, just above upper line I will drop you screenshot

And why did you comment out if (Radio.hasData())? This will make it so radio data is written to the FIS every loop, even if it's old data. The "request" code will never trigger, if the radio decides to stop sending data for some reason. And if somehow, it doesn't send the "off" message when you turn it off (happens on my radio, seen with logic analyzer), the radio data will remain in the FIS because the "request" code with the 5 attempts doesn't happen.

my mistake, I was investigating why code with your line do not initialize RADIO MODE communication with my testing jig, Strangly sometime it works sometimesnot, I try real radio and fix jig code, or move it to your lib :D

tomaskovacik commented 4 months ago

It wasn't really a "decision". XOR mode is a side effect of two bits being changed in the mode byte, which would overwrite the INVERTED and TRANSPARENT options.

The real problem is, I can't find time to write proper documentation. That is what I wanted to do in the Wiki section of the Github page. So that's true, I left it without explanation.

self-explanatory functions name do not need Wiki pages ;)

Maybe I should separate setDrawColor into setTextColor and setBitmapColor, so then I could add setBitmapMode(XOR) without affecting the text color. And even leave setDrawColor and make it call those two functions to keep the original behavior for old code.

XOR in text mode makes no sense that is true, I will try to understand, maybe bitmap mode has totally different meaning that is posibility

domnulvlad commented 4 months ago

I am shipping out modules based on atmega88 and with that huge bitmap it just did not fit

You could fit an Audi logo in 64x22 pixels which is 176 bytes instead of the 704 bytes your logo has.

self-explanatory functions name do not need Wiki pages ;)

Definitely. But behavior should still be explained, for example that setting drawColor once will make all following draws have the same setting. And how workspaces are used, etc. These have been explained in the demos, but still, a Wiki would be nice.

now I realize XOR is in TEXT section, now it makes no sense :D

What are you referring to?

tomaskovacik commented 4 months ago

I am shipping out modules based on atmega88 and with that huge bitmap it just did not fit

You could fit an Audi logo in 64x22 pixels which is 176 bytes instead of the 528 bytes your logo has.

self-explanatory functions name do not need Wiki pages ;)

Definitely. But behavior should still be explained, for example that setting drawColor once will make all following draws have the same setting. And how workspaces are used, etc. These have been explained in the demos, but still, a Wiki would be nice.

now I realize XOR is in TEXT section, now it makes no sense :D

What are you referring to?

I deleted that comment, XOR is mentioned in the document I have in graphics mode also (I did no push F3 enough times :) )https://github.com/tomaskovacik/VAGFISWriter/blob/83322335c3d227fa9fb5bb15b735150a4f97fd23/src/VAGFISWriter.cpp#L386

domnulvlad commented 4 months ago

Ok, so here is some testing of the drawColor and transparency options, in all combinations. First, I cleared the left half of the screen with the INVERTED color, to make it red, then drew text and a bitmap on top.

OPAQUE, NORMAL: black and red pixels drawn directly on top image

TRANSPARENT, NORMAL: black pixels make background come through image

OPAQUE, INVERTED: red pixels are drawn as black, black pixels make background come through image

TRANSPARENT, INVERTED: (XOR mode) red pixels invert the background, black pixels make background come through image

Then, leaving the text and bitmap on TRANSPARENT, INVERTED, I drew another bitmap on top, from the middle of the screen, down.

OPAQUE, NORMAL image

TRANSPARENT, NORMAL image

OPAQUE, INVERTED image

TRANSPARENT, INVERTED image

Do you think I should rename the functions to something else so it makes more sense?

tomaskovacik commented 4 months ago

well if you write it in TRANSPARENT, INVERTED order then it make more sense,

in transparent mode, zeroes are ignored and only 1 is taken in consideration in inverted mode then when 1, what is on the cluster is flipped

but only if you do not think about inverted "colour" because that in my head is still 1->black 0->red not something 1->flig 0->do nothing

tomaskovacik commented 4 months ago

http://boni.ygd.pl/viewtopic.php?t=600664

domnulvlad commented 4 months ago

This is the table I made for all combinations. curr is the bitmap's pixel, prev is the pixel already on the screen, res is the resulting pixel

Mode 10 (OPAQUE,NORMAL) => res=curr
  1 1 = 1
  0 1 = 0
  1 0 = 1
  0 0 = 0
Mode 11 (TRANSPARENT,NORMAL) => res=curr|prev
  1 1 = 1
  0 1 = 1
  1 0 = 1
  0 0 = 0
Mode 00 (OPAQUE,INVERTED) => res=(!curr)&prev
  1 1 = 0
  0 1 = 1
  1 0 = 0
  0 0 = 0
Mode 01 (TRANSPARENT,INVERTED) => res=curr^prev
  1 1 = 0
  0 1 = 1
  1 0 = 1
  0 0 = 0