Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Apr 12 16:11

    kobalicek on master

    [Bug] Fixed invalid move constr… (compare)

  • Apr 02 19:35
    nside commented #108
  • Apr 02 19:34
    nside commented #108
  • Apr 02 19:10
    nside commented #108
  • Apr 02 19:10
    nside commented #108
  • Apr 02 15:30
    nside commented #108
  • Apr 02 14:59
    nside commented #108
  • Apr 02 09:49
    kobalicek commented #108
  • Apr 02 06:19
    nside commented #108
  • Apr 02 05:56
    nside opened #108
  • Mar 31 21:47
    kobalicek commented #107
  • Mar 31 21:46
    kobalicek commented #107
  • Mar 31 15:40
    AndrewAPrice opened #107
  • Mar 24 23:13
    kobalicek commented #106
  • Mar 24 20:56
    electroduck opened #106
  • Mar 23 00:31
    kobalicek commented #105
  • Mar 23 00:24
    kobalicek commented #105
  • Mar 22 04:40
    justnope commented #105
  • Mar 22 04:31
    justnope commented #105
  • Mar 21 04:34
    justnope edited #105
Petr Kobalicek
@kobalicek
Glad it works for you :)
Yeah perspective transformations are on my list!
Robert M. Münch
@Robert-M-Muench

Is this sequence OK?

img.Init -> ctx.Init -> img.Create -> img.Create -> img.Destroy -> ctx.Destroy

Or do I have to do anything explicit with the ctx after an img.Create?

Petr Kobalicek
@kobalicek
Try to see it like this - Init is constructor, destroy is destructor, create is method
I don't think the sequence is wrong
but why to do img.Create twice if it's the same image?
Robert M. Münch
@Robert-M-Muench
It’s just an example, of course the images would be different.
My point was about the ctx and if it needs to know about the img changes.
Petr Kobalicek
@kobalicek
it keeps weak reference to the image
if you change the target image, context would still see the previous image
there is no way you can switch buffer of the rendering context like this
Robert M. Münch
@Robert-M-Muench
Sp, the correct sequence would be:
img.Init -> ctx.Init -> img.Create & ctx.Init -> img.Create & ctx.Init -> img.Destroy -> ctx.Destroy
Petr Kobalicek
@kobalicek
BLImage img(...);
BLContext ctx;

ctx.create(img);
img.reset();

// Ctx still has the original image, and the original image is not destroyed.
// It's an insurance so it won't have an invalid buffer in case this happens.
Robert M. Münch
@Robert-M-Muench
BTW: These things are IMO the biggest pitfalls with B2D because the documentation doesn’t contain anything about those side-effects or internal implicit actions.
Petr Kobalicek
@kobalicek
but I still don't understand what is the problem - Init constructs, Destroy destructs, the rest are methods that can only be called on constructed object
Well it's not really a side effect, it's just an insurance
you cannot really deallocate memory that can still be used by worker threads
Robert M. Münch
@Robert-M-Muench
I’m talking about something different. Init/Destroy is not the problem. I’m talking about these when used in combination with img & ctx.
Petr Kobalicek
@kobalicek
when you attach a rendering context to the image, the image must be kept alive until you detach it - if you accidentally destroy the image earlier, the rendering context would prevent deallocation of the buffers - it would basically postpone the destruction of the image until it detaches
Robert M. Münch
@Robert-M-Muench

Yes, I understand that worker-thread stuff. Again, it’s that as a user of B2D I don’t have any chance to find out that B2D behave like this. It’s not wirtten anywhere:

If you do img.Create and have a ctx attached, the ctx keeps an internal copy of the image before img.Create, the new img is not attached to any ctx.

Petr Kobalicek
@kobalicek
it's kinda a special case to make sure that you cannot trick Blend2D to use dangling pointer to an image
Robert M. Münch
@Robert-M-Muench
And, yes, I think it needs to be that detailed and explicitly formulated.
Petr Kobalicek
@kobalicek
it's basically the same behavior as with patterns and gradients
if you set a gradient to the context - it will basically copy it, so even if you change it later it won't change the internal copy that the rendering context holds. Again - it's not possible to change something that the rendering context is using, possibly across threads
I will document this better thanks for the suggestion
Robert M. Münch
@Robert-M-Muench
I think a simple block picture how these things are related makes it clear. Again, it’s not that I don’t understand why things are the way they are. It’s just not obvious where such a pattern is used, and where not.
Does img.Create clear the bitmap or does it contain random stuff?
Petr Kobalicek
@kobalicek
No it doesn't clear it
That would make no sense
as allocating large buffer would mean clearing it in a single thread during create()
Robert M. Münch
@Robert-M-Muench
No problem, again, I just need to know that I have to take care about this myself.
Sergei
@cbum_gitlab

@kobalicek Maybe add a function:

BLResult blGlyphBufferMove(BLGlyphBufferCore* self, BLGlyphBufferCore* other) noexcept {
  BLInternalGlyphBufferImpl* impl = blInternalCast(self->impl);
  self->impl = blInternalCast(other->impl);
  other->impl = nullptr;

  if (impl != &blGlyphBufferInternalImplNone)
    impl->destroy();
  return BL_SUCCESS;
}

and move operator to class blGlyphBuffer:

BL_INLINE BLGlyphBuffer& operator=(BLGlyphBuffer&& other) noexcept { blGlyphBufferMove(this, &other); return *this; }

To be able to create an array of BLGlyphBuffer, for example:

Array<BLGlyphBuffer> arrGlyphs;
arrGlyphs.append(BLGlyphBuffer());
Petr Kobalicek
@kobalicek
Good suggestion. I think I will make it compatible with BLVariant at some point as well
Sergei
@cbum_gitlab

@kobalicek In glyphbuffer.cpp in function:

BLResult blGlyphBufferInitMove(BLGlyphBufferCore* self, BLGlyphBufferCore* other) noexcept {
  BLInternalGlyphBufferImpl* impl = blInternalCast(self->impl);
  other->impl = const_cast<BLInternalGlyphBufferImpl*>(&blGlyphBufferInternalImplNone);
  self->impl = impl;
  return BL_SUCCESS;
}

in first string ERROR, must be:

BLInternalGlyphBufferImpl* impl = blInternalCast(other->impl);

other->impl not self->impl
Right?

Sergei
@cbum_gitlab
And why old self not destroyed?
Sergei
@cbum_gitlab

Also probably need to check for NULL:

if (impl != nullptr && impl != &blGlyphBufferInternalImplNone)
    impl->destroy();

impl != nullptr
What do you think about it?

Petr Kobalicek
@kobalicek
Impl is never null - that's why internal "None" impl is there
This simplifies the design greatly as it's guaranteed that no impl is ever null on a constructed object
This applies to all Blend2D containers
But I see a bug in blGlyphBufferInitMove
Petr Kobalicek
@kobalicek
And I have fixed it
Sergei
@cbum_gitlab
Understood. Thank you.
Petr Kobalicek
@kobalicek
btw move assignment is a good idea
I will check it out once I get a bit of time
And think whether it would be possible to make the impl compatible with other impls
Sergei
@cbum_gitlab
In my test program, I have already added a move assignment. And it works great.
Sergei
@cbum_gitlab

Impl is never null ...

BLGlyphBuffer* pGlyphs = new BLGlyphBuffer[10];
memset(pGlyphs, 0, 10 * sizeof(BLGlyphBuffer));
delete[] pGlyphs;

What will happen? :)

Petr Kobalicek
@kobalicek
replace BLGlyphBuffer with std::map<int, int>
It's definitely undefined behavior if you change the content of a class outside its API