INPStarfall / Starfall

Starfall Processor for Garry's Mod + Wiremod
http://www.wiremod.com/forum/developers-showcase/22739-starfall-processor.html
Other
17 stars 15 forks source link

Holo scale incorrect on certain models #219

Open AlexanderArvidsson opened 10 years ago

AlexanderArvidsson commented 10 years ago

I've been testing around with models and I've found that scaling holos that has the model set to any of the hologram models will scale them incorrectly, There may be more models that this happens on, but I have only found that the models from "models/holograms/*.mdl" scales incorrectly. Any other model works fine (I have yet to find another one that does not work, so I assume it is only the hologram models), and I used a phx model to demonstrate that, and phx is not the only model that scaling works on.

Here are some pictures to show what I mean: These are set to scale 4,1,1 with holoScaleUnits (my own function in starfall, since none exists) The E2 holo are always on the left side. http://i.imgur.com/sdJPFlG.png (Correct) http://i.imgur.com/LpPZwTe.png (Incorrect) Here are some with normal setScale from both e2 and starfall. http://i.imgur.com/vtjJzlr.png (Correct) http://i.imgur.com/jU1g9mc.png (Incorrect)

Increasing the scale on the Y axis will increase the scale on the X, as well as increasing X will increase Y too. The Z axis remains unnaffected by X and Y, and works just as expected.

Some more examples on this: http://i.imgur.com/zpms5Vn.png (E2 on the left, scale at 2,2,1) http://i.imgur.com/MW6BVZr.png (Scale is at 1,3,1) http://i.imgur.com/WXXSwa8.png (Scale is at 2,5,1)

Before you say anything, there is nothing wrong with my holoScaleUnits function, as shown with the normal models. However, the scaling is wrong when using setScale on hologram models (Which is what my holoScaleUnits function does, except it calculates the unit scale from their OBB).

zoombahh commented 10 years ago

Since this should be read by more people than chatspam on inp.io:

In e2 (rather, the gmod_wire_hologram entity) the scaling has an explicit exception for hologram models. See below. https://github.com/wiremod/wire/blob/master/lua/entities/gmod_wire_hologram.lua

local count = self:GetBoneCount() or -1
if count == 1 then
    for i = count, 0, -1 do
        local bone_scale = self.bone_scale[i] or scale
        if string.Left(self:GetModel(), 17) == "models/holograms/" then
            bone_scale = Vector(bone_scale.y, bone_scale.x, bone_scale.z)
        end
        self:ManipulateBoneScale(i, bone_scale)
    end
elseif etc. etc. 

Under the assumption that GetBoneCount() returns 1 for the e2 models. this means that they would get scaled by using self:ManipulateBoneScale(i.Vector(y,x,z)). Note the swapped X and Y axis, and that the bones get explicitely scaled when count == 1.

awilliamson commented 10 years ago

Seems to me that this is wiremods fault and we shouldn't have to add an exception for their models. Our users should be able to do this given that they know the differences. I haven't studied this much as I'm away right now, I'd like to get @xandaros opinion on this.

zoombahh commented 10 years ago

If we had the tools to do it ourselves I would implement the exception myself. Can we scale bones from SF?

As always the "userspace" workarounds also pose a hurdle to new / uninformed users who stumble upon the same old issues. On the other hand I agree that implementing wire hacks/exceptions should be carefully evaluated and avoided where possible.

awilliamson commented 10 years ago

As far as I'm aware all normal models work, it's just wiremods that are special. Yes it poses a barrier for new players but they won't be wanting to use wiremod models and it would then be common knowledge that you have to have a work around to use them models. Such exceptions even exist within wiremod itself.

shadowscion commented 10 years ago

This is wiremod's fault, they have switched back and forth between XYZ and YXZ several times since GMod13 hit. Please don't change the way starfall works to accommodate their inconsistency. If you really want it to work that way, you can do it yourself from within starfall.

AlexanderArvidsson commented 10 years ago

The thing is, we can't do it ourselves, since we don't have a way to change the scaling to use ManipulateBoneScale. The problem is not that X and Y are flipped, the problem is that it uses the 'wrong' scaling function for those models.

I have started to use sprops models anyway so fixing this no longer affects me either way.

awilliamson commented 10 years ago

Then may I recommend you go raise an issue on their github repo. It's not our fault their models require this odd exception to be scaled. If people desperately need these models then somebody could remake them and propose their addition into Starfall. You can use any model for our holos, they're just entities. People should learn to expand their palette when it comes to using holos, especially when transitioning from E2.

AlexanderArvidsson commented 10 years ago

At the time of opening this issue I was not sure if it only happened on wires models, and I thought it was a good idea to report all issues I could find.

The only difference these models have is that their OBB box is exact which means scaling them to units is precise, unlike sprops, phx or any other model. If you look at the very first picture, you can see a gap inbetween which is caused by a not precise OBB box around the model. This gap also gets wider the larger you want to scale the model. No, it's not caused by my scaleUnits function.

As I said above; I am using sprops now so it doesn't matter, this issue can be closed if no one got something to add.

shadowscion commented 10 years ago

@Metamist What scaling function should it be using, then?

shadowscion commented 10 years ago

@Metamist ManipulateBoneScale is the only serverside function that can scale models by a vector, I am not even sure why e2 holos even have the EnableMatrix code in there.

zoombahh commented 10 years ago

Starfall does:

--- Sets the hologram scale
-- @param scale Vector scale
function ENT:SetScale ( scale )
    self.scale = scale
    local m = Matrix()
    m:Scale( scale )
    self:EnableMatrix( "RenderMultiply", m )

    local propmax = self:OBBMaxs()
    local propmin = self:OBBMins()

    propmax.x = scale.x * propmax.x
    propmax.y = scale.y * propmax.y
    propmax.z = scale.z * propmax.z
    propmin.x = scale.x * propmin.x
    propmin.y = scale.y * propmin.y
    propmin.z = scale.z * propmin.z

    self:SetRenderBounds( propmax, propmin )
end

while the gmod_wire_hologram scaling code uses three methods for maximum compabnillity. only ONE of theese methods applies to the wire holo models and includes the xy Hack. copypasta from the already linked https://github.com/wiremod/wire/blob/master/lua/entities/gmod_wire_hologram.lua

function ENT:DoScale()
    local eidx = self:EntIndex()

    if scale_buffer[eidx] ~= nil then
        self.scale = scale_buffer[eidx]
        scale_buffer[eidx] = nil
    end

    if bone_scale_buffer[eidx] ~= nil then
        for b, s in pairs(bone_scale_buffer[eidx]) do
            self.bone_scale[b] = s
        end
        bone_scale_buffer[eidx] = {}
    end

    local scale = self.scale or Vector(1, 1, 1)

    local count = self:GetBoneCount() or -1
    if count == 1 then
        for i = count, 0, -1 do
            local bone_scale = self.bone_scale[i] or scale
            if string.Left(self:GetModel(), 17) == "models/holograms/" then
                bone_scale = Vector(bone_scale.y, bone_scale.x, bone_scale.z)
            end

            self:ManipulateBoneScale(i, bone_scale)
        end
    elseif self.EnableMatrix then
        local mat = Matrix()
        mat:Scale(Vector(scale.x, scale.y, scale.z))
        self:EnableMatrix("RenderMultiply", mat)
    else
        -- Some entities, like ragdolls, cannot be resized with EnableMatrix, so lets average the three components to get a float
        self:SetModelScale((scale.x + scale.y + scale.z) / 3, 0)
    end

    local propmax = self:OBBMaxs()
    local propmin = self:OBBMins()
    self:SetRenderBounds(Vector(scale.x * propmax.x, scale.y * propmax.y, scale.z * propmax.z), Vector(scale.x * propmin.x, scale.y * propmin.y, scale.z * propmin.z))
end

as you may (or may not) see, wire uses the same method IF (and only if) self:GetBoneCount() ~= 1 and self.EnableMatrix is truthy. If the bone count is 1 it scales the bones nr 1 and 0 directly (this is also where the x y swap comes in). If EnableMatrix is not there it defaults to scaling the whole thing to the average given, presumably in this case SF's scaling would simply fail.

If SF at least adopted the seperate bone scaling (without the XY swapping) OR there was a seperate boneScale function we could simply use swapped X and Y in "userspace", something everyone can figure out for theese models. As things are now (SF always using EnableMatrix, even when there is exactly one bone and/or no EnableMatrix) and without an alternate bone scaling function we simply cannot use theese models effectively.

Personally I like the sound of something like boneScale(boneIndex, scaleVector) because that would be an actual improvement AND allow us to use theese models.

Also just as a sidenote I disagree about moving to other models as the holo models are made specifically for that purpose, offer a variety of geometrical shapes all to one scale, with accurate bounding boxes. also they look decent even with scaling and stretching.

EDIT: just saw your second comment @shadowscion They are all using clientside code. both SF and wire send the scale to the client by net message in between. Also they have enablematrix for props with 0 or >1 bone, since e2 holograms CAN be used with arbitrary models (if enabled).

shadowscion commented 10 years ago

Okay I tested it out for myself, and I have to agree... something is definitely wrong.

local sf = entities.self()
local holo = holograms.create(sf:getPos(), sf:getAngles(), "models/holograms/cube.mdl", Vector(1, 12, 1))

Using a scale of 1, 12, 1 with the 12x12x12 cube model, you would expect a 12 wide, 144 long, 12 tall box. Instead, this is what you get:

image

Oddly, it works perfectly fine on the hologram models that I contributed to wiremod (rcube, rcylinder, cplane). My best guess is something weird was done when the others were modeled, but I have no idea what it could be, all of the models only have one bone.

Xandaros commented 10 years ago

As long as only 3rd party models are affected and we are forced to use hacks to remedy this, I am not inclined to do anything about it. It works perfectly fine with any stock model and it also works fine with SProps, so it has to be wire's fault. Or Garry's, but at this point that is unlikely.

You can change the material of the an sprops prop and make it look just like a wire hologram, I think. If that doesn't work, maybe someone can make new hologram models that work properly and don't require ugly hacks like that.

@Metamist As I said before, it was a good thing of you opening this issue. It is a perfectly valid issue, as this can be considered a compatibility problem, but I don't feel like starfall needs to adapt to wire's oddities (And you didn't know it was just wire's models.) If you find another model that is available in gmod without addons, or sprops, we'll look into it, but as it stands I don't think we should do anything.

@zoombahh I don't know why they check if enableMatrix exists. As far as I know there is not a single instance where it doesn't. If you find one, please open an issue :)

awilliamson commented 10 years ago

I agree with @Xandaros. This is wire's compatibility due to some oddity in their base hologram models. As previously said, somebody could make some base holograms for us if they wished, looking at you @shadowscion, and we'd take a look at it, but SProps works great for most applications.

zoombahh commented 10 years ago

Would there be a downside to implementing the "use ManipulateBoneScale() if there is exactly one bone" solution or (alternatively) allowing SF to access ManipulateBoneScale() and GetBoneCount() ?

Im not talking about the xy swap. that is something users should be able to cope with and a dirty hack on WM's part. I dont see what would be wrong with scaling the first bone if there is only one. Having the ability to correctly scale these models (read: X and Y separately) would be nice.

AlexanderArvidsson commented 10 years ago

The thing is, using sprops or any cubic model still has its own issues, because wires model is the only model I have come across that returns precise OBB size. Me and shadow just now figured out that many of sprops models OBBox is offset by 0.3-0.5, while phx models offsets are even weirder and more inconsistent. Now this is most likely a GMod issue, but not being able to use the wire holo models makes our scaling inprecise when using their OBB size for unit scaling. Only way to fix it is to not scale to units, or hardcoding the OBB sizes for each model used.

Giving us the ability to scale bones in the model will allow us to fix it ourselves in our userspace. It would also let us scale ragdoll bones separately, i.e. City Scanner for more creative builds.