Closed DaleSchultz closed 5 years ago
leading me to believe that updateDisplayArea() is broken
Ahm well... It is not broken, but I think I missed to describe this. The tile coordinates of updateDisplayArea()
apply to the unrotated (U8G2_R0) display.
u8g2.setFlipMode(1)
does the rotation on hardware level
Indeed, it rotated the display OK but, only the top half of the display worked when using updateDisplayArea() i.e. 4 of the 8 tile rows display only.
oh, this is a bug... I need to check this.
OK great, thanks for taking a look.
I would like to make a constructive suggestion for you kind consideration
I think that it makes sense from an API usability point of view that the tile coordinates also flip with U8G2_R2, otherwise we have to produce code that places something in tile row 2 and has to refresh tile row 5 to make it display! To do that, the code has to track if we are rotated or not, or else it is not portable. I suspect even your sample code at https://github.com/olikraus/u8g2/blob/master/sys/arduino/u8g2_full_buffer/UpdateArea/UpdateArea.ino would have to to updated for it to work with U8G2_R2 too! It would make much more sense to do that in the library than (in my case) amateur code.
It will be less code maintenance for me to keep changing setFlipMode and never use the constructor rotation.
Another thought: If setFlipMode does the rotation at the hardware level, why not use that capability for U8G2_R2 (when available)?
BTW I initially implemented a different library for SSD1306 and then when I ordered my SSD1322 display, I found u8g2 and I was very impressed by the documentation in the wiki and extensive hardware support. I decided to redo all my implementation and I am very impressed by it all. I am sure you tend to only hear feedback when there is an issue, so I want to express that appreciation here. Great job!
To do that, the code has to track if we are rotated or not, or else it is not portable.
True
I suspect even your sample code ... would have to to updated for it to work with U8G2_R2 too!
Yes of course.
Another thought: If setFlipMode does the rotation at the hardware level, why not use that capability for U8G2_R2 (when available)?
Ok, I try to answer this: U8G2_Rx rotation is a pure software solution and it is always available for U8g2 API. In fact there is no such rotation for U8x8 API. In contrast, the setFlipMode is hardware based and also available in the U8x8 API. It also does not require any computation effort and also the extra code is very small. Disadvantage: Not all displays do support this.
Basically users will select the display and the API:
U8x8 API + Display without setFlipMode support: Physically rotate your display, no rotation support by this lib U8x8 API + Display with setFlipMode support: Use setFlipMode for rotation U8g2 API + Display without setFlipMode support: Use U8G2_Rx constructor rotation U8x8 API + Display with setFlipMode support: You can choose between Use U8G2_Rx constructor rotation and setFlipMode. Performance and code size can be used as selection criteria.
Again your question:
If setFlipMode does the rotation at the hardware level, why not use that capability for U8G2_R2 (when available)?
There is always a risk from making things too smart. Beeing smart in a lib context always means, that extra code is required. If you use U8G2_R0, then the code for U8G2_R2 is not included into your code, saving some bytes in the flash ROM. However, it U8G2_R2 should be smart in the sense that it should fall back to U8G2_R0 if setFlipMode is available or not, then the code for both R0 and R2 is required at runtime, occupying valuable resources in your controller. This runtime selection will require, that code for both methods must be included in your executable, making your code bigger.
If course I could go further and try to move this selection back to the u8g2 constructor construction tool (yes, all the constructors are automatically generated), but this will be a huge effort.
My goal was: Keep it simple and do all the required selection at compile time. I think this is a fundamental difference between a windows 10 library and an embedded systems library. In a desktop environment, it does not matter, whether your code is 10MB or 20MB, in Arduino World it is a big difference.
I think that it makes sense from an API usability point of view that the tile coordinates also flip with U8G2_R2, otherwise we have to produce code that places something in tile row 2 and has to refresh tile row 5 to make it display!
Well, I have to reply with the same argument: Do users what a smart (but fat) library or a simple (and small) library. It fits to the same concept as above: If you use U8G2_R2 you have to manually change the update area. Even mathematically it will be a little bit complicated. But you have to do it once at compile time. No code is needed in your flash. Obviously, it would be more comfortable if I add some code to do the rotation related calculation in U8g2 lib. But then the code is there, probably useless for all those users who do not require U8G2_R2 but occupying flash ROM.
I have two users: Those who complain about the size and those who complain about the API comfort.
At the moment I put more priority on the code size, but try to compensate this with a good API documentation.
So as a result: My suggestion is to define two sets of constants which hold the arguments for the UpdateArea command. Depending on the applied rotation you select a different set of these constants. This implementation should be easy with #if conditional compile commands. This will make your code portable, uses the existing API and minimizes your code.
Great job!
Thanks a lot!
Note: Check u8g2.setFlipMode(1)
with SSD1322
I did a test with u8g2 page buffer example FlipMode.ino. This is the constructor:
U8G2_SSD1322_NHD_256X64_1_4W_HW_SPI u8g2(U8G2_R0, /* cs=*/ 10, /* dc=*/ 9, /* reset=*/ 8); // Enable U8G2_16BIT in u8g2.h
setFlipMode
works as expected.
yes, as I reported, setFlipMode works fine, which is what I am now using.
I understand the different demands of the API and such trade-offs.
As to not needing conversion logic code at run time, that is only true if the text and locations is not somewhat hardcoded and perhaps that is the typical use. I am building a system that takes dynamic text, and locations from a network connection, and displays them at run time. The number of rows of text is not fixed either, so all text positions and update areas have to be calculated at run time. (I already have a function to convert from pixel coordinates to tiles to enable this.)
It produces a very flexible system and when I publish it I will also be giving credit for the libraries used. Many thanks.
Sounds promising. I would be interested to see this once..
my e-mail is there in each example and source code headers...
closing...
I have code that works well with SSD1306 using I2C
Today I got a NewHaven 256x64 display and got it connected up (4 wire SPI). Initially, I had a problem with
updateDisplayArea
being offset, but I fixed that by enabling 16 bit mode.Everything looked fine, but I wanted the display the other way up, so I changed the rotation to U8G2_R2:
U8G2_SSD1322_NHD_256X64_F_4W_HW_SPI u8g2 (U8G2_R2, D8, D2, D1);
Indeed, it rotated the display OK but, only the top half of the display worked when usingupdateDisplayArea()
i.e. 4 of the 8 tile rows display only.
In addition, they draw in the reverse order, I draw lines 0, 1, 2, 3, 4, 5 , & 6 (8 pixels each) and I see them drawing in order 3, 2, 1, & 0 (rows 4 through 6 never appeared, but am guessing they drew first).
If I switch back to using
u8g2.updateDisplay();
the display is correctly rotated and displays in the correct order, leading me to believe thatupdateDisplayArea()
is broken when combined with U8G2_R2 and full buffer on SSD1322 256x64Using
u8g2.setFlipMode(1);
instead ofU8G2_R2
works fine, (even withupdateDisplayArea()
) but I would prefer to have the flip defined in the constructor so that I don't have to remember to change code every time I build for a different display.