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

inventory encumbrance labels can get hidden #59

Closed
lynxlynxlynx opened this issue Aug 21, 2017 · 15 comments
Closed

inventory encumbrance labels can get hidden #59

lynxlynxlynx opened this issue Aug 21, 2017 · 15 comments

Comments

@lynxlynxlynx
Copy link
Collaborator

I thought perhaps clicking on the bag button below them forces it infront, however just making the button locked or fakepressed doesn't fix this. Making it disabled helps, even though it's the wrong state, but it doesn't fix it completely and sometimes the labels clip the button bam ...

Could be some new code is not checking for all the button states (2x locked and 2x "fake"). I don't think there's any reason not to draw labels on the toppest top.

Likely also happens with other created labels. All that I can think of are currently untestable: encumbrance on the container window, in stores, hp overlay in iwds.

@bradallred
Copy link
Owner

the order of views isnt going to change in response to any sort of event. The order of subview drawing is the order they were added, but that shouldn't matter if things are built right. It sounds like the button and labels are both being added to the window rather than the button being added to the window and the label then being added to the button resulting in overlapping subviews of the window.

I'd have to dig in to actually see whats happening, but if its what i think it is it should be an easy fix.

@lynxlynxlynx
Copy link
Collaborator Author

@bradallred
Copy link
Owner

bradallred commented Aug 22, 2017

so does changing Window.CreateLabel to Button.CreateLabel fix it then?

presumably the r value needs to change to be relative to the button too.

@lynxlynxlynx
Copy link
Collaborator Author

haven't tried yet, didn't have much time.

@lynxlynxlynx
Copy link
Collaborator Author

It helps, but then another problem pops up. If you click on any items, the button (bag) starts disappearing.

@bradallred
Copy link
Owner

We should probably add some debugging aides to detect overlapping subviews. I don't know what else it could be unless clicking those trigger a callback that is manipulating the button.something like: Inside the loop that iterates subviews add another loop and check if any intersections exist and draw a red border around them.

if we have to we can add code to redraw them, but I was trying to be efficient and redraw as little as possible and we should avoid that if possible (and I believe it is).

@lynxlynxlynx
Copy link
Collaborator Author

It's true that with the change, the labels now have the size of the button. And picking an item up changes encumbrance, so their content changes. The labels have no background, but how is the renderer supposed to know only the bottom or top part where the text is needs redrawing? Still, it should be transparent, so why the effect on the button?

There's two of them, but we could probably reduce it to one subview with some textual padding. Not as nice or robust, but better than nothing.

@bradallred
Copy link
Owner

bradallred commented Aug 23, 2017

are you sure that IsOpaque() is returning false for those labels?

Is the "button" even a button? can we just make it a generic view with a background? I suspect that would fix it (a button with a background instead of an image should work too).

I suppose this is loaded as-is from a CHU?

@bradallred
Copy link
Owner

bradallred commented Aug 23, 2017

to answer your question about why it is effecting the button: it isn't (and thats the problem). The label change marks it for redraw, but that doesnt subsequently mark the button for redraw.

the next redraw cycle will redraw everything starting from the root view (window). which will draw its background over everything. it will then draw its subviews and encounter the button (which isn't marked as dirty) since it doesnt need to redraw it just tells its subviews to redraw causing the dirty label to redraw. end result makes it appear the button has vanished.

The foundation exists for marking regions as dirty, but none of the control drawing methods take advantage. They all assume they are always redrawing their entire contents so it's something I was saving for later.

short of implementing partial drawing for controls (draw only the portion of their images that was covered) the only options that come to mind are:

  1. change the image to a background
  2. implement a new overload for MarkDirty() that takes a rect (precursor to partial redraws). the difference being that this method would call MarkDirty(rect) on its subviews (only if they intersect the passed rect) and pass the intersection rect. we then invoke this new overload from MarkDirty() in the if (superView && !IsOpaque()) case.
  3. simply add superView->MarkDirty() to the if (superView && !IsOpaque()) case inside MarkDirty(), but this basically means any change to any control causes everything to redraw so its a poor choice.
  4. have View::DirtyBGRect iterate the subviews and mar intersecting ones as dirty

@lynxlynxlynx
Copy link
Collaborator Author

Yes, it's a button already in the chu. How do you change it to a background, recalling SetBAM? SetPicture is for bmps, then we have one for sprites — quite a mess.

But I don't fully understand: if the background window is fully redrawn, everything has to be drawn again to be visible, so why bother with dirty views at all? It sounds like we have 3. already.

@bradallred
Copy link
Owner

bradallred commented Aug 23, 2017

SetBackground() works with any sprite, but if this is from CHU and not something we created then it's moot.

I need to amend my statement about controls redrawing everything. Background drawing is handled by the View base and it does only redraw dirty regions, not the entire BG.

So the window isn't fully redrawn every frame, but if the label is a non opaque direct subview of the window then the portion of the window background that the label occupies will be redrawn. since this portion also overlaps the button the background is drawn over both the button and label, but only the label gets redrawn over the background since the button isn't "dirty".

we never redraw the whole window/background unless the window itself is marked dirty (which causes everything to be marked dirty and hence redraw completely).

This is what is happening currently.

If you have moved the labels to be subviews of the button then something else is happening since the labels would then tell the button to redraw its background segment rather than the window and the window shouldn't be redrawing that segment of background without one of its direct decedents being marked as dirty. I would look to see if there are any other unknown controls in play.

@lynxlynxlynx
Copy link
Collaborator Author

Nothing interesting in the CHU, the bag is there on its own.

@bradallred
Copy link
Owner

I looked into this and its still the same essential problem.

the label dirties the background of the button, which recursively dirties the background segment of the windows since the button is considered transparent.

as an aside:
apparently there is no picture on the button, but rather a "picture list" (which is why it is not considered opaque). I don't understand why this is a picture list or why we have 2 variable for pictures rather than a single list.

@lynxlynxlynx
Copy link
Collaborator Author

Don't know the history, but I bet we have picture list support for the pause button — it has to display two or three images at a time. Just do a git log -p -Gvarname and jump to the first commit to see.

But why wouldn't a picturelist button be considered opaque? That also seems wrong.

bradallred added a commit that referenced this issue Aug 23, 2017
invalidating the background shouldn't be recursive, but should mar us as dirty
possibly other uses that shouldn't be recursive

should fix issue #59 and probably #60
@bradallred
Copy link
Owner

I'm going to open a new issue for the IsOpaque stuff. images can have an alpha so just checking if they exist was a lazy thing to do an is incomplete.

bradallred pushed a commit that referenced this issue May 22, 2018
README: Fix link to forum
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants