Skip to content
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

Add support for different bitmap scaling qualities (BitmapInterpolationQuality) #64

Merged
merged 4 commits into from Jul 12, 2018

Conversation

Projects
None yet
2 participants
@raygit83
Copy link
Contributor

raygit83 commented Jul 6, 2018

The bitmap scaling quality can be set per CDrawContext instance using the CDrawContext::setBitmapInterpolationQuality() or globally per CFrame instance using CFrame::setBitmapInterpolationQuality(), which then delegates the setting to the respective draw context in CFrame::drawRect().

Currently, there are three modes: kQualityDefault, kQualityLow, kQualityMedium and kQualityHigh.

This feature is transparent, i.e. nothing will change from the current behavior unless you set anything != kQualityDefault.

NOTE: A Gdiplus back end implementation was skipped given that Gdiplus will deprecated soon. Linux support is missing due to the lack of a test setup. Since the older direct2d API which d2ddrawcontext is based on only offers two modes (nearest neighbour and bilinear), the kQualityMedium and kQualityHigh settings both map to the same mode on Windows.

Reimund Dratwa
Add support for different bitmap scaling quality settings (BitmapInte…
…rpolationQuality).

The bitmap scaling quality can be set globally, e.g. in your editor's ctor, e.g.: CDrawContext::interpolationQuality = CDrawContext::kQualityHigh.

Currently, there are three modes: kQualityDefault, kQualityLow and kQualityHigh.

This feature is transparent, i.e. nothing will change from the current behavior unless you set anything != kQualityDefault.

NOTE: A Gdiplus back end implementation was skipped given that Gdiplus will deprecated soon. Linux support is missing due to the lack of a test setup.
@scheffle

This comment has been minimized.

Copy link
Collaborator

scheffle commented Jul 6, 2018

Hi ray,
it's problematic to use statics here, as a draw context could be used in one thread to draw into a bitmap context and in one thread to draw into the window and you may want to use different interpolation settings. I would prefer to make this a state settings of a context instance.
Cheers,
Arne

@raygit83

This comment has been minimized.

Copy link
Contributor Author

raygit83 commented Jul 6, 2018

Hi Arne,

It was a conscious design decision to make it a static. It only affects the way bitmap are rendered and if you think about it, as a plugin programmer would you really need different quality settings for different bitmaps? Keeping that design pattern in mind, I think it's unproblematic in a multi threaded environment, too. Making it a CDrawContext state would make it become effectively inaccessible unless you implement your own draw() method in a Control. An alternative could be moving it into IPlatformBitmap / CBitmap (?) given that it's a property that affects bitmap drawing?

Best,
Ray

@scheffle

This comment has been minimized.

Copy link
Collaborator

scheffle commented Jul 6, 2018

How about adding a property on the CFrame object. The CFrame instance can then setup the draw context when it receives the platformDrawRect() call from the platform implementation. This way you just call once frame->setBitmapInterpolationDrawMode (..) after constructing it. My use case is also possible to draw high quality into a bitmap context while drawing low quality on to the screen.

@raygit83

This comment has been minimized.

Copy link
Contributor Author

raygit83 commented Jul 6, 2018

Hi Arne,

I just so happened to think about moving it into CFrame, too. So yes, I agree with your suggestion. I'll rearrange this PR to include the proposed change asap.

@raygit83

This comment has been minimized.

Copy link
Contributor Author

raygit83 commented Jul 6, 2018

Hi Arne,

referring to your previous use case I noticed that it's impossible to use COffscreenContext's capability to render into a bitmap when you're using CLayeredViewContainers on Mac. I already addressed this on the VSTGUI Forum way back but never received any response. I had two instances, one in which I was trying to show a blurred/grayscale version of the CFrame content using the provided CBitmapFilters another one in which I essentially needed a getPixel() for a color picker/eye dropper, and rendering into a bitmap simply did not work when working with CLayeredViewContainers. I can search for some example code for reproduction if you like.

@raygit83

This comment has been minimized.

Copy link
Contributor Author

raygit83 commented Jul 9, 2018

Hi Arne,

I have integrated the changes we discussed. Please check my update.

Best,
Reimund

@@ -70,14 +70,16 @@ class CDrawContext : public AtomicReferenceCounted
// @name Bitmap Interpolation Quality
//-----------------------------------------------------------------------------
//@{
enum BitmapInterpolationQuality
enum CBitmapInterpolationQuality

This comment has been minimized.

@scheffle

scheffle Jul 10, 2018

Collaborator

Please make this an enum class and move it into the VSTGUI namespace. Thanks.

@@ -179,7 +183,8 @@ class CFrame final : public CViewContainer, public IPlatformFrameCallback
//-------------------------------------------
protected:
struct CollectInvalidRects;

CDrawContext::CBitmapInterpolationQuality bitmapQuality;

This comment has been minimized.

@scheffle

scheffle Jul 10, 2018

Collaborator

Please move member variables into the impl, thanks.

@@ -7,6 +7,7 @@

#include "vstguifwd.h"
#include "cviewcontainer.h"
#include "cdrawcontext.h"

This comment has been minimized.

@scheffle

scheffle Jul 10, 2018

Collaborator

remove this include by adding CBitmapInterpolationQuality into the vstguifwd.h header, thanks.

@scheffle

This comment has been minimized.

Copy link
Collaborator

scheffle commented Jul 10, 2018

Hi Reimund,
thank you for this. Please make the changes I commented inline and everything is ready to merge :-)

@scheffle

This comment has been minimized.

Copy link
Collaborator

scheffle commented Jul 10, 2018

Regarding your other issue with CLayeredViewContainers, please provide some example code, and maybe add this to #12.

@raygit83

This comment has been minimized.

Copy link
Contributor Author

raygit83 commented Jul 11, 2018

Hi Arne,
I added the changes you suggested. Regarding the CLayeredViewContainer issue, I'll add this to #12 asap (after my holidays that is). It's pretty easy to reproduce though: In an overrider of CView::draw(), create a CBitmap in the view's size, create an COffscreenContext with that bitmap. Then use tha COffscreenContext to do perform any drawing you would otherwise do on the CDrawContext passed to draw and finally blit the result into the output context. Expected result: The CBitmap holds the contents drawn into the COffscreenContext. Actual result: The Bitmap is empty if the CView under consideration is embedded in a CLayeredViewContainer.

@scheffle
Copy link
Collaborator

scheffle left a comment

Looks good, just one more change. Every new enum should be a c++11 enum class to prevent wrong usage.

//----------------------------
// @brief Bitmap Interpolation
//----------------------------
enum BitmapInterpolationQuality

This comment has been minimized.

@scheffle

scheffle Jul 11, 2018

Collaborator

change this to : enum class BitmapInterpolationQuality
and change the members to kDefault,kLow,kMedium,kHigh.

This comment has been minimized.

@raygit83

raygit83 Jul 11, 2018

Author Contributor

Okay, I tried to stick to the other enums in vstguifwd.h which aren't enum classes, either. Any specific reason why that is?

This comment has been minimized.

@scheffle

scheffle Jul 11, 2018

Collaborator

I don't want to break old code if it is not necessary.

This comment has been minimized.

@raygit83

raygit83 Jul 11, 2018

Author Contributor

Okay, thanks, that makes sense.

This comment has been minimized.

@raygit83

raygit83 Jul 11, 2018

Author Contributor

I know this is off-topic, but is there any specific reason that CFrame has been marked final? Performance?

I have an instance where I need to override some functions in CFrame in order to access its pixel contents using the COffscreenContext/CBitmap approach described above. This is done to implement an eyedropper. What would be the suggested workaround here?

This comment has been minimized.

@scheffle

scheffle Jul 11, 2018

Collaborator

The workaround would be to place everything into a root CViewContainer that you override for this case.

This comment has been minimized.

@raygit83

raygit83 Jul 11, 2018

Author Contributor

That makes perfect sense. Thank You!

@raygit83

This comment has been minimized.

Copy link
Contributor Author

raygit83 commented Jul 11, 2018

Hi Arne, I have just applied your suggested changes.

@raygit83

This comment has been minimized.

Copy link
Contributor Author

raygit83 commented Jul 12, 2018

I have added a code example to #12 as discussed.

@scheffle
Copy link
Collaborator

scheffle left a comment

Super, thanks.

@scheffle scheffle merged commit 9842ca9 into steinbergmedia:master Jul 12, 2018

scheffle added a commit to scheffle/vstgui that referenced this pull request Aug 14, 2018

Merge pull request steinbergmedia#64 from raygit83/master
Add support for different bitmap scaling qualities (BitmapInterpolationQuality)

scheffle added a commit that referenced this pull request Sep 21, 2018

Merge pull request #64 from raygit83/master
Add support for different bitmap scaling qualities (BitmapInterpolationQuality)

scheffle added a commit that referenced this pull request Jan 2, 2019

Merge pull request #64 from raygit83/master
Add support for different bitmap scaling qualities (BitmapInterpolationQuality)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.