Skip to content

[G-API] Implement render using stateful kernel#18600

Merged
alalek merged 6 commits intoopencv:masterfrom
TolyaTalamanov:at/implement-render-using-stateful
Oct 26, 2020
Merged

[G-API] Implement render using stateful kernel#18600
alalek merged 6 commits intoopencv:masterfrom
TolyaTalamanov:at/implement-render-using-stateful

Conversation

@TolyaTalamanov
Copy link
Copy Markdown
Contributor

@TolyaTalamanov TolyaTalamanov commented Oct 16, 2020 •
edited
Loading

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

Build configuration

force_builders=Custom
buildworker:Custom=linux-1
build_image:Custom=ubuntu-gapi-freetype:16.04

build_gapi_standalone:Linux x64=ade-0.1.1f
build_gapi_standalone:Win64=ade-0.1.1f
build_gapi_standalone:Mac=ade-0.1.1f
build_gapi_standalone:Linux x64 Debug=ade-0.1.1f

@TolyaTalamanov TolyaTalamanov changed the title [G-API] Implement render using stateful kernel [WIP][G-API] Implement render using stateful kernel Oct 16, 2020
@TolyaTalamanov TolyaTalamanov force-pushed the at/implement-render-using-stateful branch from 7a0d0ad to 95b2eac Compare October 19, 2020 09:54
@TolyaTalamanov TolyaTalamanov force-pushed the at/implement-render-using-stateful branch from 95b2eac to 0f2e247 Compare October 19, 2020 10:10
@TolyaTalamanov TolyaTalamanov changed the title [WIP][G-API] Implement render using stateful kernel [G-API] Implement render using stateful kernel Oct 19, 2020
@TolyaTalamanov TolyaTalamanov force-pushed the at/implement-render-using-stateful branch from 00b8963 to 1ba6d3c Compare October 19, 2020 12:22
@TolyaTalamanov
Copy link
Copy Markdown
Contributor Author

TolyaTalamanov commented Oct 21, 2020

@dmatveev

dmatveev
dmatveev approved these changes Oct 22, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing code is always nice!

Viva la stateful kernels, @AsyaPronina should be proud :)

modules/gapi/src/backends/render/grenderocv.cpp Outdated

if (has_freetype_font)
{
state->ftpr = std::make_shared<FTTextRender>(has_freetype_font.value().path);
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev Oct 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

->path

Copy link
Copy Markdown
Contributor Author

@TolyaTalamanov TolyaTalamanov Oct 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't get your point

Copy link
Copy Markdown
Contributor Author

@TolyaTalamanov TolyaTalamanov Oct 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

modules/gapi/src/backends/render/grenderocv.cpp Outdated
const cv::GCompileArgs& args)
{
using namespace cv::gapi::wip::draw;
auto has_freetype_font = cv::gapi::getCompileArg<freetype_font>(args);
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev Oct 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while this is valid in terms of optional<>, this may be hard to understand by a non-prepared person

Copy link
Copy Markdown
Contributor Author

@TolyaTalamanov TolyaTalamanov Oct 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

modules/gapi/src/backends/render/grenderocv.cpp Outdated
auto has_freetype_font = cv::gapi::getCompileArg<freetype_font>(args);
state = std::make_shared<RenderOCVState>();

if (has_freetype_font)
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev Oct 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...so I'd write it this way:

const auto opt_freetype_font = get<>();
if (opt_freetype_font.has_value()) {
    ...
}

Copy link
Copy Markdown
Contributor Author

@TolyaTalamanov TolyaTalamanov Oct 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@TolyaTalamanov
Copy link
Copy Markdown
Contributor Author

TolyaTalamanov commented Oct 26, 2020

@alalek

@alalek alalek merged commit 3d45639 into opencv:master Oct 26, 2020
@alalek alalek mentioned this pull request Nov 27, 2020
Merged
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
@TolyaTalamanov
Merge pull request opencv#18600 from TolyaTalamanov:at/implement-rend…
79de974 
…er-using-stateful

[G-API] Implement render using stateful kernel

* Implement render using stateful kernel

* Move freetype to backends folder

* Fix freetype compilation

* Fix freetype smoke test

* Fix comments

* Refactoring
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmatveev dmatveev dmatveev approved these changes

Assignees

@dmatveev dmatveev

Labels

category: g-api / gapi feature

Projects

None yet

Milestone

4.5.1

Development

Successfully merging this pull request may close these issues.

3 participants

@TolyaTalamanov @dmatveev @alalek