-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Framebuffer #1
base: 4.x
Are you sure you want to change the base?
Framebuffer #1
Conversation
CMakeLists.txt
Outdated
@@ -297,6 +297,10 @@ OCV_OPTION(WITH_GTK "Include GTK support" ON | |||
OCV_OPTION(WITH_GTK_2_X "Use GTK version 2" OFF | |||
VISIBLE_IF UNIX AND NOT APPLE AND NOT ANDROID | |||
VERIFY HAVE_GTK AND NOT HAVE_GTK3) | |||
|
|||
OCV_OPTION(WITH_FRAMEBUFFER "Include framebuffer support" OFF | |||
VISIBLE_IF UNIX VERIFY HAVE_FRAMEBUFFER) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CMake variable HAVE_FRAMEBUFFER
is not set anywhere, so our verification process will fail (see ENABLE_CONFIG_VERIFICATION
description (https://docs.opencv.org/4.x/db/d05/tutorial_config_reference.html#autotoc_md913).
Maybe we should use more strict condition targeting not UNIX in general, but Linux specifically. E.g. UNIX AND NOT APPLE AND NOT ANDROID
maybe even CMAKE_SYSTEM_NAME EQUAL "Linux"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommended restrictions added
modules/highgui/CMakeLists.txt
Outdated
@@ -39,6 +39,17 @@ if(HAVE_WEBP) | |||
add_definitions(-DHAVE_WEBP) | |||
endif() | |||
|
|||
|
|||
if(WITH_FRAMEBUFFER) | |||
message(WITH_FRAMEBUFFER="${WITH_FRAMEBUFFER}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to add separate block to the common configuration output in root CMakeLists.txt, at the end of "GUI" section (https://github.com/opencv/opencv/blob/26a5730f0d36f547a6eaf39b1e15786ea3f0d039/CMakeLists.txt#L1367-L1425).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The message output has been moved to the root CMakeLists.txt
|
||
int FramebufferBackend::fb_open_and_get_info() | ||
{ | ||
int fb_fd = open("/dev/fb0", O_RDWR); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also allow changing default device name. As I understand common environment variable used to set it is FRAMEBUFFER
, additionally we can check our custom variable, e.g. OPENCV_HIGHGUI_FRAMEBUFFER
(highest priority).
cv::utils::getConfigurationParameterString
function and static function (singletone) can be used for this (example).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added reading of the corresponding environment variables
int fb_fd = open("/dev/fb0", O_RDWR); | ||
if (fb_fd == -1) | ||
{ | ||
std::cerr << "ERROR_OPENING_FB\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OpenCV logging facilities should be used (example).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stdout has been replaced by the OpenCV logging system
return; | ||
} | ||
|
||
fb_w = var_info.xres; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to allow only supported FB configurations (check bits_per_pixel/grayscale/red/green/blue/transp/colorspace/...?) and log error message + return gracefully for unsupported configurations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modules/highgui/test/test_gui.cpp
Outdated
@@ -70,6 +70,9 @@ TEST(Highgui_GUI, regression) | |||
|
|||
EXPECT_NO_THROW(destroyAllWindows()); | |||
ASSERT_NO_THROW(namedWindow(window_name)); | |||
#if defined HAVE_FRAMEBUFFER | |||
ASSERT_NO_THROW(resizeWindow(window_name, 800, 600)); | |||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this is correct in terms of OpenCV logic. Without resizing the window, the window and image sizes are not the same.
9593c9b
to
2e9cbf6
Compare
Реализация framebuffer в OpenCV